Add support for time.Time flags #348

max-frank · 2022-03-23T11:47:35Z

This PR implements support for time.Time flags which can be used to accept timestamps as input directly (see spf13/cobra#742).

The implementation allows defining a list of acceptable timestamp formats which are tested in order, i.e., devs using this flag type would be responsible for ordering their formats correctly should there be an overlap.

kostrahb · 2022-04-27T20:02:51Z

This is exactly what I am looking for. Would it be possible to review and merge this?

Maybe also suggestion for small improvement, would it be possible to offer some reasonable standard default ordering?
Like this:

var DefaultOrdering []string = []string{time.RFC3339, time.UnixDate, ...}

or this:

func getDefaultOrdering() []string {
	return []string{time.RFC3339, time.UnixDate, ...}
}

max-frank · 2022-05-06T04:09:14Z

I was thinking of adding a constant slice with a suggested default time format set, but opted to not include it in this PR since its easy to define such a constant in whatever code you use and more importantly I wanted to avoid any long winded discussions on what the actual order and included formats should be.

AnthonyMBonafide

This looks great! Can't wait to get this out and into Cobra.

max-frank · 2022-08-21T08:33:50Z

@johnSchnake tagging you (a bit late) for this PR as requested in spf13/cobra#742

This PR implements support for `time.Time` flags which can be used to accept timestamps as input directly (see spf13/cobra#742). The implementation allows defining a list of acceptable timestamp formats which are tested in order, i.e., devs using this flag type would be responsible for ordering their formats correctly should there be an overlap. Merge spf13#348

alexferrari88 · 2023-07-19T10:57:15Z

hate being that guy...but: any news on when will this be in?

max-frank · 2023-07-19T11:07:04Z

Yeah sadly seems that the repo is just sitting here now anymore nothing happening :/

i tried to make some noise in the related issue on the cobra side but nothing yet

mmontes11 · 2023-12-18T16:05:55Z

Anything needed from the community side to cross the finish line with this PR? Happy to test, review, anything... it is a bit of a shame that the de-facto standard CLI library doesn't have a time.Time flag like it has already for time.Duration

Kapparina · 2024-08-24T07:03:43Z

Good to see this has been approved... Can it be merged?

max-frank · 2024-09-05T06:57:39Z

Good to see this has been approved... Can it be merged?

@Kapparina i wish it was this easy :(

AnthonyMBonafide

This looks good to me.

IvanRibakov · 2024-10-10T17:46:20Z

Anyone knows who needs to be chased for the approved PR to be merged in this repo? Is it only @spf13 or do more people have merge rights?

max-frank · 2025-07-11T21:57:55Z

@tomasaschan Sorry for the direct ping, but going by recent activity you seem to have the ability to review and approve PRs. Any chance you'd have some time to look at this 🙇

tomasaschan · 2025-07-12T18:49:40Z

time_test.go

+	devnull, _ := os.Open(os.DevNull)
+	os.Stderr = devnull


Why is this needed? What in this test would otherwise write to stderr?

For this one going back by memory I believe only reason I've added it was because of copy paste from the test file I modified to make it easier to stick to the repo style.

That is one of these

https://github.com/search?q=repo%3Aspf13%2Fpflag+%09os.Stderr+%3D+devnull&type=code

I've checked now and as far as I can tell non of the occurrences of this in the repo are doing anything.
Removed it for time_test.go in d15848d

Thanks for the quick response to the review request 🙇

tomasaschan · 2025-07-12T18:54:28Z

time_test.go

+	testCases := []struct {
+		input    string
+		success  bool
+		expected string


Nit: I think I'd prefer if this was a time.Time rather than a string, to make it easier to add other test cases without having to dig into how comparisons are done and what format to specify dates in. But I'm not going to block on that :)

Updated in c5ce22e

max-frank · 2025-07-13T23:58:31Z

Note during rebase to master I had to rename setUpTime to setUpTimeVar due a conflict introduced by new code

whatsacomputertho · 2025-07-14T16:17:27Z

What a coincidence, I stumbled upon this issue earlier today as I was interested in this feature

time.Time flags cobra#742

and it seems like this is nearing completion! Fantastic! Thank you Frank for staying persistent on this PR.

time.go

tomasaschan

A couple of more nits; I added suggestions for them so I can commit them with you as co-author and get this merged, I hope you don't mind :)

time.go

CLAassistant · 2025-07-16T21:37:45Z

All committers have signed the CLA.

tomasaschan · 2025-07-16T21:42:36Z

Thanks for your contribution!

max-frank · 2025-07-17T01:06:06Z

A couple of more nits; I added suggestions for them so I can commit them with you as co-author and get this merged, I hope you don't mind :)

Yeah no worries I am happy to have this modified to fit the new direction 👍

Thanks for the quick support and review after the ping 🙇

AnthonyMBonafide reviewed May 6, 2022

View reviewed changes

fredbi approved these changes Nov 21, 2022

View reviewed changes

gssbzn mentioned this pull request Apr 28, 2023

CLOUDP-171854 Add commands atlas datalakepipelines availablesnaphosts and availableschedules mongodb/mongodb-atlas-cli#1921

Merged

6 tasks

max-frank mentioned this pull request Jun 15, 2023

time.Time flags spf13/cobra#742

Open

AnthonyMBonafide approved these changes Sep 5, 2024

View reviewed changes

tomasaschan reviewed Jul 12, 2025

View reviewed changes

Add support for time.Time flags

1992c5a

max-frank force-pushed the add-time-flag-support branch from 7e4fa0a to 1992c5a Compare July 13, 2025 23:36

max-frank added 2 commits July 14, 2025 08:44

Use time.Time for expectations in time flag tests

c5ce22e

Remove unnecessary time test stderr dev null redirect

d15848d

max-frank requested a review from tomasaschan July 15, 2025 04:40

tomasaschan reviewed Jul 16, 2025

View reviewed changes

time.go Outdated Show resolved Hide resolved

tomasaschan reviewed Jul 16, 2025

View reviewed changes

Don't export TimeValue (yet)

7cc25e3

tomasaschan approved these changes Jul 16, 2025

View reviewed changes

Reduce duplication by forwarding to sibling functions

e3be2eb

tomasaschan force-pushed the add-time-flag-support branch from e8e90db to e3be2eb Compare July 16, 2025 21:40

tomasaschan approved these changes Jul 16, 2025

View reviewed changes

tomasaschan merged commit f9cbdd9 into spf13:master Jul 16, 2025
5 checks passed

renovate bot mentioned this pull request Jul 17, 2025

fix(deps): update misc non-major dependencies ido-namely/docker-protoc#33

Open

1 task

tomasaschan mentioned this pull request Jul 17, 2025

implement CopyToGoFlagSet #330

Open

Add support for time.Time flags #348

Add support for time.Time flags #348

Conversation

max-frank commented Mar 23, 2022

Uh oh!

kostrahb commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

max-frank commented May 6, 2022

Uh oh!

AnthonyMBonafide left a comment

Choose a reason for hiding this comment

Uh oh!

max-frank commented Aug 21, 2022

Uh oh!

alexferrari88 commented Jul 19, 2023

Uh oh!

max-frank commented Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmontes11 commented Dec 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kapparina commented Aug 24, 2024

Uh oh!

max-frank commented Sep 5, 2024

Uh oh!

AnthonyMBonafide left a comment

Choose a reason for hiding this comment

Uh oh!

IvanRibakov commented Oct 10, 2024

Uh oh!

max-frank commented Jul 11, 2025

Uh oh!

tomasaschan Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

max-frank Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

max-frank Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

tomasaschan Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

max-frank Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

max-frank commented Jul 13, 2025

Uh oh!

whatsacomputertho commented Jul 14, 2025

Uh oh!

Uh oh!

tomasaschan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CLAassistant commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tomasaschan commented Jul 16, 2025

Uh oh!

max-frank commented Jul 17, 2025

Uh oh!

Uh oh!

kostrahb commented Apr 27, 2022 •

edited

Loading

max-frank commented Jul 19, 2023 •

edited

Loading

mmontes11 commented Dec 18, 2023 •

edited

Loading

CLAassistant commented Jul 16, 2025 •

edited

Loading