-
-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for showing start time in 24-hour format #78
Add support for showing start time in 24-hour format #78
Conversation
main.go
Outdated
@@ -178,6 +187,7 @@ var manCmd = &cobra.Command{ | |||
func init() { | |||
rootCmd.Flags().StringVarP(&name, "name", "n", "", "timer name") | |||
rootCmd.Flags().BoolVarP(&altscreen, "fullscreen", "f", false, "fullscreen") | |||
rootCmd.Flags().BoolVarP(&twentyFourHourStartTime, "24", "", false, "Use 24-hour format for start time") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it would be nicer to do something like:
--format 24h
--format kitchen
etc
that way we could add more formats in the future... or even allow the user to provide their own.
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I thought about it, but was a bit reluctant to leak implementation-specific details such as "kitchen" meaning that specific format. But if you prefer it, I could modify the PR...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, I'll squash the fixup commits before potential merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comment, other than that LGTM 🙏🏻
main.go
Outdated
case "kitchen": | ||
startTimeFormat = time.Kitchen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be removed, handled by default
Thank you! |
Solves #55