Open fruitraccoon opened 5 years ago
Yeah, the formatter is a little dumb about negatives. It's saying "(-1):(-30)" because it doesn't know how to pull the negative out as "-(1:30)"
@icambron
Can I take this issue?
So In this example I just need to print in "-(1:30)" format, right, when user use toFormat("hh:mm")
and pass hh:mm
as parameter ?
@chinmaydalvi I think there's some thinking required here, because it's a little under-specified. There are a few issues to sort out:
I welcome everyone's thoughts on this, but frankly I'm tempted toward 3.
I had a bit of a think about it and a bit of a look at the code. Formatting certainly does not seem like the easiest thing to change.
To address the comments from @icambron above:
- Units can be mixed, with some negative and some positive. How do we handle that?
We really just need to know if the total of the duration is less than 0 - if it is, then a single -
can just be added "somewhere". But should it be to the left of the left-most value in the format string? Perhaps there needs to be a format string symbol so that it can be placed explicitly?
Maybe there's some guidance that can be taken from the standard libraries of other languages in how they handle formatting negative durations.
- Do we really want parens? I put them in to illustrate, but I'm not sure if they're really the best thing to show, since they aren't in the format string.
No, I agree that we'd only want to show characters that are in the format string.
- Is this worth fixing, or should we just say "if you want nuanced negative handling, you're on your own"?
It would be good to get it fixed if possible. That said, the following is a workaround:
const testValue = Duration.fromObject({ days: -1, minutes: 59 });
const testValueMs = testValue.as('milliseconds');
const isNeg = testValueMs < 0;
const positiveTestValue = Duration.fromMillis(Math.abs(testValueMs));
const formatted = positiveTestValue.toFormat(`${isNeg ? '-' : ''}hh:mm`);
// formatted: "-23:01"
My solution:
function isNegativeDuration (duration) {
return duration.valueOf() < 0
}
function formatDuration (duration, pattern) {
if (isNegativeDuration(duration)) {
return `-${duration.negate().toFormat(pattern)}`
} else {
return duration.toFormat(pattern)
}
}
formatDuration(Duration.fromObject({ minutes: 1 }), 'h:mm') // 0:01
formatDuration(Duration.fromObject({ minutes: -1 }), 'h:mm') // -0:01
On mixed-sign values:
We really just need to know if the total of the duration is less than 0 - if it is, then a single - can just be added "somewhere".
This is not that simple. The first problem is that finding out it's negative requires converting between units. That's no big deal as long as we respect the conversionAccuracy
setting (which toMillis
does), but it's a little strange. The second problem is to be useful, we need all the units to have the same sign, whichever one we decided the total is. This should be the job of normalize
, but it currently does not always do that:
> Duration.fromObject({ months: -1, days: 30, hours: -5}).normalize().toObject()
{ months: 0, days: -1, hours: 19 }
So I think we need to fix normalize to take care of that. Then we just check if the toMillis() < 0
and if so, normalize.
But should it be to the left of the left-most value in the format string? Perhaps there needs to be a format string symbol so that it can be placed explicitly?
I'd prefer we just make a convention for it, like -([your format])
you can apply normalize until nothing is to be normalized anymore:
let normalized = value.normalize();
while (!normalized.equals(value)) {
value = normalized;
normalized = value.normalize();
}
all components end up being positive or negative, not pretty but it does the job
- Units can be mixed, with some negative and some positive. How do we handle that?
I am not sure what is the use-case of mixing units with different signs, I am sure there are still. Do you have an example? In moment.js this has been discussed (https://github.com/moment/moment/issues/2408) but it's not part of any spec if I understood well.
What if we just threw errors if users try to provide units with different signs?
As for how I encountered this error, I tried to do:
Duration.fromObject({minutes: -200}).forFormat("hh:mm");
And the output was: -3:-20
instead of my own expectation of -03:20
(just like GMT offsets).
TL;DR: no heuristics in a library, please!
I know this is an old issue (and a tough one), but I thought it might be interesting to share my experience with durations. I have worked in the insurance industry for 25 years now, where date arithmetic is core business. Insurance policies need all sorts of date/duration calculations that compute future/past effective dates of several events like expiry, premium due dates, etc. These use different duration units (years, quarters, months and days are the most frequent ones). Most systems have very (i.e. think insanely) sophisticated logic - mostly coded as a mess :) , btw.
I am not sure what is the use-case of mixing units with different signs, I am sure there are still.
Yes, there are. One very simple example would be "calculate the last day of a policy year starting on day x", which corresponds to a date / duration pseudo-expression of x + (+1 year -1 day)
. Other, much more complex scenarios also exist. Mixing positive and negative components in a duration is definitely a valuable feature of a library.
My experience can be summed up as
My personal analysis/opinion/preference on the original request:
Note the formatting option being used: .toFormat("hh:mm")
. This says nothing about sign. Nevertheless, hiding the possible sign in the produced output is obviously not a good choice.
This example is special in the sense that minutes / hours can always be exactly converted. It is much better to think about units that cannot be converted consistently (like year / day, month / day, etc.). For example all of the following are meaningful and possible durations: -1 year -1 day
, -1year +1 day
, +1 year -1 day
, +1 year +1 day
. Out of those, the request would apply to -1 year -1 day
To me it seems arbitrary (and unwanted) to have some "hidden heuristic" that results in a "magic sign" jumping somewhere (where exactly?) for one specific case only, especially if we consider the possible set of format strings.
BTW (somewhat related to this issue): to me even the fact that .toFormat()
does converting and normalization behind the scenes is unwanted. This is precisely the kind of heuristic which ultimately makes programmers start recoding behavior as part of the business logic. For example:
Duration.fromObject({year:1, day:-1}).toFormat("y d") // "0 364"
Such behavior results in loss of information. The internal Duration object is +1 year -1 day
, not 0 year +364 day
. Also, it is sometimes inaccurate as 1 (calendar !) year is either 365 or 366 days long.
> Duration.fromISO('PT1M1S').negate().toString() 'PT-1M-1S'
Is there any plans to support sign?
When
toFormat("hh:mm")
is used on a negativeDuration
, the resulting string can have extra minus characters.Example:
Example is available here: https://codesandbox.io/s/43wry09vw4
Version 1.10.0