franciscolourenco / done

A fish-shell package to automatically receive notifications when long processes finish.
MIT License
770 stars 70 forks source link

Make the time display more easily parsable by humans #17

Closed skorokithakis closed 7 years ago

skorokithakis commented 7 years ago

Since the notification is only shown after 10 seconds, I think the milliseconds display is not very useful. Instead, since this will only be shown for long-running commands, I have changed the format to look like 01:54:22.

This fixes #15.

franciscolourenco commented 7 years ago

On macOS 10.11.6, no time is displayed. This is printed in the console.

$ sleep 11
usage: date [-jnu] [-d dst] [-r seconds] [-t west] [-v[+|-]val[ymwdHMS]] ...
            [-f fmt date | [[[mm]dd]HH]MM[[cc]yy][.ss]] [+format]
skorokithakis commented 7 years ago

Hmm, I'm afraid I don't have an OS X machine. Can you debug the date command? I'm using date -d "1970-01-01 30 seconds" +%H:%M:%S, which works fine on Linux.

franciscolourenco commented 7 years ago

Have a look at maybe you can spot the difference https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man1/date.1.html

skorokithakis commented 7 years ago

Looks like you may need date -d "1970-01-01 30 seconds" "+%H:%M:%S", although then that won't work on Linux... Bah.

franciscolourenco commented 7 years ago

That still doesn’t work on mac though.

skorokithakis commented 7 years ago

Sorry, I meant date -d "1970-01-01 30 seconds" -f "+%H:%M:%S"

franciscolourenco commented 7 years ago

Still the same

skorokithakis commented 7 years ago

Alright, I think this is done. It should work on all OSes, and the humanization is great. I'm just not sure whether it currently pulls the dependency in correctly, but I think so.

franciscolourenco commented 7 years ago

Works well, thanks!

A few things before merging:

  1. Squash the commits into one
  2. Don't use period in the commit title
  3. Should we also remove the period from the notification title?

EDIT: Just added https://github.com/fisherman/done/blob/master/CONTRIBUTING.md

skorokithakis commented 7 years ago

You can squash and edit the merge commit message when merging the PR, Github shows a dialog. About the period in the title, I can remove it if you want.

franciscolourenco commented 7 years ago

Merged, thanks!

PS: In the future follow the just now added CONTRIBUTING.md

skorokithakis commented 7 years ago

Will do, thanks!