franciscolourenco / done

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

Add __done_humanize_duration, remove fishfile #91

Closed IlanCosman closed 3 years ago

IlanCosman commented 3 years ago

This way is pure fish and removes dependencies. It is also much faster.

ammgws commented 3 years ago

I guess this would be one way to go especially since there's no indication that https://github.com/fishpkg/fish-humanize-duration/pull/2 will ever be merged.

franciscolourenco commented 3 years ago

This sounds good to me, thank you @IlanCosman and @ammgws ā€“ have you tested these changes?

IlanCosman commented 3 years ago

Yes, I have tested it. It doesn't display milliseconds though, but I can add that if you wish.

franciscolourenco commented 3 years ago

@IlanCosman I'm not even sure if it is useful to know the ms, but I suppose it wouldn't hurt either. Any takes on this @ammgws?

ammgws commented 3 years ago

@franciscolourenco If it is made to correctly handle durations over 60 hours then it should be good to go. That same bug exists with humanize_duration itself, and investigating that is what led me to rewrite it using fish builtins in https://github.com/fishpkg/fish-humanize-duration/pull/2.

See wrong times given when the duration is longer than 60 hours:

>__done_humanize_duration 23000000
6h 23m 20sāŽ
>__done_humanize_duration 230000000
3h 53m 20sāŽ
IlanCosman commented 3 years ago

Fixed šŸ˜„ Modding by 60 for hours was completely ridiculous anyways. It's 24 hours to a day not 60 šŸ˜‚

franciscolourenco commented 3 years ago

Thank you for the contribution!