franciscolourenco / done

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

Humanize the time display #15

Closed skorokithakis closed 7 years ago

skorokithakis commented 7 years ago

It would be nice if instead of "1837 seconds", the display said "30:37".

franciscolourenco 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 is interesting but there might be more human ways than Finished in 00:00:11. What do you think of 02h40m11s? Ideally if it less than a minute, 11s. If it is less than an hour 3m15s. Or 3min 15sec.

skorokithakis commented 7 years ago

I definitely like that more, but you know what they say, a PR in the hand is worth two issue requests in the bush, so I figured I'd try my hand at it. I'm not great at fish scripting, so I couldn't easily implement what you describe, or I would have.

I think a good compromise would be to merge this version (because it's better than "354.391s") and keep the issue open for the improved version?

jorgebucaran commented 7 years ago

There's a fisherman plugin that could help with that.

skorokithakis commented 7 years ago

Oh, that looks ideal, thank you!

franciscolourenco commented 7 years ago

@jbucaran does fisherman support dependencies?

skorokithakis commented 7 years ago

I'll just copy it over, it's just an awk function.

skorokithakis commented 7 years ago

That won't work, that plugin prints the duration rather than return it.

jorgebucaran commented 7 years ago

@skorokithakis In fish (other shells too?) you can't return values from functions, but rather capture the output sent to the different standard streams.

~ set -l human_time (echo 10000 | humanize_duration)
~ echo $human_time 
10s
~ 
skorokithakis commented 7 years ago

Oh, thank you. I'll give it a go, then.

skorokithakis commented 7 years ago

Should be done, works fine for me! Let me know if there's anything else you need before merging the PR.