Timer/countdown, a few fixes, some cleanup (manpage, option orders)

[MoonCactus]

tty-clock is both good and small, which rocks! Still, I wanted to use it also as a pomodoro timer/countdown ...

It should still be perfectly compatible with your version. There are a few new commands (arguments -p, -e and options D, E, Z, space), a pair of fixes (abusive border with x with -D, locale #66 ) ... and eventually some non-insignificant cleanup (the manpage, -h and README are now consistent, and more meaningfully ordered IMHO).

Admittedly, I might have better asked for multiple PR, but I wanted to move forwards today, and I wanted to rewrite my fork so it gets hopefully easier to track for you (I re-ordered the commits for smaller steps and more readability).
Every commit below should be atomic, stable and progressive.

So you have it in one package here. I hope you'll like it (I kept it as small as possible, and that it gets merged in your repo ! :)

[anarcat]

oh wow, great! awesome code! :) it would be nice, however, if you could split that up a little. i suspect that there are much less controversial PRs (like the patch for #66) which we would just breeze through, while some others (like disabling the hour? why?) are more controversial.

i understand you have made a specific effort to split those up in neat little commits, and that's great. but it still makes the whole thing hard to review, as github doesn't provide a great interface to review commit-per-commit, unfortuantely. i also doubt you want me to start picking apart the entire thing all at once, since I can't merge only parts of it... :p

but thanks again for the PR! maybe some more lenient hand will magically come and merge this instead of me, but that's my stance on this for now. ;)

[MoonCactus]

Thanks for your feedback & I am glad you like the code ;)

I understand it is not the best PR you could get from me... Actually I really needed a "pomodoro" and was fed up with the larger, bloated ones... Then I did not feel like waiting at each step since I had a clear view of what I wanted :o
Sorry, I am not accustomed with github enough to know if/how you could just check parts of the PR and now I know it is not that easy :/

I still tried not to impact anything in place (except for the cleanup and semantic categorization of the options in the man page & getopt). Eg. regarding disabling the hour : it is really an option that was meant for -e and -p and which must be set explicitly (it is not set by default). The order of my commits makes it weird indeed. It simply spares screen space for shorter-than-one-hour timers and countdowns. Eg. I split my time in 35-minute chunks and having a constant "00:" on screen was pretty not minimalist (btw, showing seconds also are an option, which on the contrary are hidden by default).
Sure, hiding the hour when using tty-clock as a clock is less interesting tty-clock still IS a clock in the first place :D

When I can find the time in the next days/week, I can re-do the fork and submit those commits in multiple PRs -- right now I need to use it to get more productive than the latest weeks. And that will make some more testing from me. Just hope I will have no more ideas (actually I have, like (re)setiting the timer via signals etc -- I will do none before we merge the fork!)

[anarcat]

On 2021-03-22 10:14:21, Jeremie Francois wrote: Thanks for your feedback & I am glad you like the code ;) I understand it is not the best PR you could get from me... Actually I really needed a "pomodoro" and was fed up with the larger, bloated ones... Then I did not feel like waiting at each step since I had a clear view of what I wanted :o Sorry, I am not accustomed with github enough to know if/how you could just check parts of the PR and now I know it is not that easy :/

I understand! :) Just think of it like this: each PR is a feature branch, work on each feature in a separate branch, and push those in your fork. The GitHub web UI (and the git push client too, even) will prompt you to create a MR based on those. Then what I usually do is to have the master branch in *my* fork merge *all* the branches I want to have in mine. Eventually i rebase my master branch on upstream, when all the branches are merged.

I still tried not to impact anything in place (except for the cleanup and semantic categorization of the options in the man page & getopt). Eg. regarding _disabling the hour_ : it is really an option that was meant for `-e` and `-p` and which must be set explicitly (it is *not* set by default). My re-ordering might make it weird indeed. It simply spares screen space for shorter-than-one-hour timers and countdowns. Eg. I split my time in 35-minute chunks and having a constant "00:" on screen was pretty not minimalist (btw, showing seconds also are an option, which on the contrary are hidden by default). Sure, hiding the hour when using tty-clock as a clock is less interesting `tty-clock` still IS a clock in the first place :D

One of my concerns with the PR is how long the usage has become. I've tried to keep people from adding new flags to the commandline because it's gotten too big for such a simple program. I don't see the point of disabling the hour at all, still. It might make sense to not show it in a timer more, but then just do that: disable it in timer mode, but don't have a flag for it. Do try to limit the number of flags you're adding, would be my overall comment I guess. :)

When I can find the time in the next days/week, I can re-do the fork and submit those commits in multiple PRs -- right now I need to use it to get more productive than the latest weeks. And that will make some more testing from me. Just hope I will have no more ideas (actually I have, like (re)setiting the timer via signals etc -- I will do none before we merge the fork!)

Makes sense! One thing you could do is to make tty-clock *exit* when the timer runs out, and then run it inside a loop in a shell script. That way you can execute arbitrary actions after the timer (ring a bell, shutdown the computer, burn the house down, etc) and restart the timer right after (or not). Try to keep things simple! :)

[MoonCactus]

I understand! :) Just think of it like this: each PR is a feature branch, work on each feature in a separate branch, and push those in your fork. The GitHub web UI (and the git push client too, even) will prompt you to create a MR based on those. Then what I usually do is to have the master branch in my fork merge all the branches I want to have in mine. Eventually i rebase my master branch on upstream, when all the branches are merged.

OK I think I got it (also, one branch per feature is pretty much what I ask for in the various projects I handle). In this case, there are a few dependencies between the PRs so I would probably have to have them approved before being able to move on the next features.

One of my concerns with the PR is how long the usage has become. I've tried to keep people from adding new flags to the commandline because it's gotten too big for such a simple program. I don't see the point of disabling the hour at all, still. It might make sense to not show it in a timer more, but then just do that: disable it in timer mode, but don't have a flag for it. Do try to limit the number of flags you're adding, would be my overall comment I guess. :)

I understand but here you probably want the hour to be shown according to how long you set the timeout (more or less than one hour). It could be automatically hidden but if its size changes automatically I think it would be annoying for ppl that set a minimalist terminal size, eg. Automatic behaviors are often right in the way.

I do agree with the minimalist stance ("do one thing well"), so let me play the devil's advocate ;) The seconds were already optional and could be toggled dynamically, so I just duplicated the idea for the hour (I would have shown both of them by default to be consistent but I did not want to break anything: seconds are hidden by default). There was a generic -f format for the date, but none for the hour. A corresponding generic & standard formatting string for the hour would have handled both the existing flags elegantly (12/24h, seconds) and not required my addition in the first place. Nb: I can give it a try if you prefer (it might require a bit of arguable filtering/parsing tho).

Also I really wished there were no such thing as the "decorative" box because it complicates different places in the code, in addition to the very useless dynamic "x" command (imo) that failed when the date was not shown and which still has a minor issue with the squeezed left-margin. And who really wants to move the clock around manually? (not to say that vim-like cursor keys fails on non-QWERTY keyboards). Changing colors dynamically also adds code :p

One thing you could do is to make tty-clock exit when the timer runs out, and then run it inside a loop in a shell script. That way you can execute arbitrary actions after the timer (ring a bell, shutdown the computer, burn the house down, etc) and restart the timer right after (or not). Try to keep things simple! :)

I pretty much agree here. Actually, it does exit when the time limit is reached :D Eg. I implemented my "pomodoro" with a shell script (it handles how tty-clock is used indeed : takes care of the terminal placement, size, transparency, and actually blinks my screen thanks to xrandr when the time is gone, before looping again).

[anarcat]

On 2021-03-22 11:54:46, Jeremie Francois wrote: [...]

I do agree with the minimalist stance ("do one thing well"), so let me play the devil's advocate ;) The seconds were already optional and could be toggled dynamically, so I just duplicated the idea for the hour (I would have shown both of them by default to be consistent but I did not want to break anything: seconds are hidden by default). There was a generic `-f format` for the date, but none for the hour. A corresponding generic & standard formatting string for the hour would have handled both the existing flags elegantly (12/24h, seconds) and not required my addition in the first place. Nb: I can give it a try if you prefer (it might require a bit of arguable filtering/parsing tho).

I think I would actually prefer supporting a generic date *and* time format than toggling it bit by bit. I wasn't so happy with the seconds toggle exactly for that reason.

Also I really wished there were no such thing as the "decorative" box because it complicates different places in the code, in addition to the very useless dynamic "x" command (imo) that failed when the date was not shown and which still has a minor issue with the squeezed left-margin. And who really wants to move the clock around manually? (not to say that vim-like cursor keys fails on non-QWERTY keyboards). Changing colors dynamically also adds code :p

Agreed. Those things were there before I joined, but it's harder to remove old stuff than to refuse new stuff. :p

[MoonCactus]

LOL, yes one has to live and respect the existing daemons :)
I think it would be hard to have one single -f formatting string for both the date and time, but two of them make sense as they are not displayed the same way.

[anarcat]

On 2021-03-22 12:06:37, Jeremie Francois wrote: LOL, yes one has to live and respect the existing daemons :) I think it would be hard to have one single `-f` formatting string for both the date and time, but two of them make sense as they are not displayed the same way.

Right, it would be two strings, unfortunately. But I still think it would make more sense than disabling the hour, because next thing you know we'll have 8 flags for date/time (and maybe more).

…

-- Any sufficiently advanced technology is indistinguishable from magic. - Arthur C. Clarke

[MoonCactus]

Right, it would be two strings, unfortunately. But I still think it would make more sense than disabling the hour, because next thing you know we'll have 8 flags for date/time (and maybe more).

Oh yes, someone will end up asking that non-significant zeroes are removed ;)
I will give it a try when I can get my hand dirty again

[Barbaross93]

Thanks so much for doing/sharing this @MoonCactus ! I've been using peaclock for a while but unfortunately I found that it was lacking some things to better incorporate into a pomodoro script that thankfully this PR addresses :). I have noticed one problem with your implementation however. If I quit the timer before it expires, the process exits succesfully. This is problematic with scripting where I have something like this:

tty-clock -p 1500 && notify-send "Time's up!"

If I quit the timer early, then I get a false alarm notification. Maybe there is a better way to script this than what I have? I suppose I could internally check the running duration of the script, but that seems needlessly complicated. Having tty-clock exit with a non-zero status when the timer ends early seems like the most elegant solution to me

[MoonCactus]

Thanks for the kind words @Barbarossa93 ! Yes it makes sense.

Having tty-clock exit with a non-zero status when the timer ends early seems like the most elegant solution to me

Actually if I let this 4 sec timer expire:

# tty-clock -sep 4; echo $?
0

Whereas if I interrupt it with control-C before the timeout:

# tty-clock -sep 4; echo $?
2

It matches the updated section EXIT STATUS in the man page: The exit status is 0 if the program is terminated either with the q command or if the timeout set with -p is reached. Exit status is 1 if an error occurred, and 2 if the program was interrupted with a signal.

So interrupting tty-clock -p 4 && echo YES will print nothing indeed?

Note that I still need to work and split this PR to get it incorporated in the main branch, but did not take the time yet :/

[Barbaross93]

Ooof, this is embarassing for me, I should've consulted the manual first! Thank you so much for pointing that out! I look forward to seeing this merged :)