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

Fix desktop notification duration formatting #4358

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

DingDongSoLong4
Copy link
Collaborator

This is a simple fix for a bug with the duration formatting in desktop notifications.

As an example, a time of 2 hours 45 minutes and 12 seconds was being formatted as 3:165:9912 rather than 2:45:12, which I believe was the original intention.

This happens because Hours(), Minutes() and Seconds() give the total duration in that unit, rather than that specific "part" of the duration. The %02.f format string specifier adds rounding as well, which makes even the hours unit incorrect half of the time.

I've just fixed this by using time.Duration's String() function, with some conditional rounding to prevent displaying tons of decimal places. This does change the output format, i.e. it's now 2h45m12s rather than 2:45:12, but I think the new format is clearer anyway, especially with times shorter than 1 second (it now shows 425ms), and it means we don't have to manually do any math ourselves.

@WithoutPants WithoutPants added the bug Something isn't working label Dec 14, 2023
@WithoutPants WithoutPants added this to the Version 0.24.0 milestone Dec 14, 2023
@WithoutPants WithoutPants merged commit 2ef2d89 into stashapp:develop Dec 14, 2023
2 checks passed
@DingDongSoLong4 DingDongSoLong4 deleted the fix-notification-fmt branch December 14, 2023 12:06
halkeye pushed a commit to halkeye/stash that referenced this pull request Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants