Skip to content
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 __done_humanize_duration, remove fishfile #91

Merged
merged 3 commits into from
Oct 10, 2020
Merged

Add __done_humanize_duration, remove fishfile #91

merged 3 commits into from
Oct 10, 2020

Conversation

IlanCosman
Copy link
Contributor

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

@ammgws
Copy link
Contributor

ammgws commented Oct 9, 2020

I guess this would be one way to go especially since there's no indication that jorgebucaran/humantime.fish#2 will ever be merged.

@franciscolourenco
Copy link
Owner

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

@IlanCosman
Copy link
Contributor Author

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

@franciscolourenco
Copy link
Owner

@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
Copy link
Contributor

ammgws commented Oct 10, 2020

@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 jorgebucaran/humantime.fish#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
Copy link
Contributor Author

IlanCosman commented Oct 10, 2020

Fixed 😄 Modding by 60 for hours was completely ridiculous anyways. It's 24 hours to a day not 60 😂

@franciscolourenco
Copy link
Owner

Thank you for the contribution!

@franciscolourenco franciscolourenco merged commit 2fdb12e into franciscolourenco:master Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants