tc39 / proposal-temporal

Provides standard objects and functions for working with dates and times.
https://tc39.es/proposal-temporal/docs/
Other
3.35k stars 153 forks source link

DifferenceISODateTime is buggy? #1868

Closed FrankYFTang closed 2 years ago

FrankYFTang commented 3 years ago

Could someone help me about this. I am lost. What should be the result of BalanceTime(-1, 0, 0, 0, 0, 0)

should it be {Days: -1, Hour:23, Minute: 0, Second: 0, Millisecond: 0, Microsecond:0 , Nanosecond:0 }

or some other value? @ptomato @Ms2ger @jugglinmike @ljharb @ryzokuken

FrankYFTang commented 3 years ago

I have hard time to understand in built-ins/Temporal/PlainDateTime/prototype/since/balance-negative-duration.js why is const earlier2 = new Temporal.PlainDateTime(2000, 5, 2, 10); const later2 = new Temporal.PlainDateTime(2000, 5, 5, 9); const result2 = later2.since(earlier2, { largestUnit: 'day' }); for the case "date sign != time sign"

and how the current spec is correct.

what I can see is BalanceTime(-1, 0, 0, 0, 0, 0) and return days=-1, hour=23, 0, 0, 0, 0, 0 and therefore both date_sign and time_sign will become 1 in

https://tc39.es/proposal-temporal/#sec-temporal-differenceisodatetime

FrankYFTang commented 3 years ago

My code follow the current spec somehow give me P1DT1H - which is surely wrong, but I have hard time to figure out where in the spec cause the problem and how to correct it. But what I can see is date_sign and time_sign in this case are both 1 but the test label they should be different. Maybe I have a bug in my code but I have hard time to figure out.

FrankYFTang commented 3 years ago

I still have problem to track down the specific issue and fix but I believe the current spec text about DifferenceISODateTime is buggy.

ptomato commented 2 years ago

I am trying to work out this case step by step, and so far I believe the current spec is correct. From the test you quoted I believe we do get dateSign and timeSign opposite:

Maybe you have the arguments to DifferenceTime in the opposite order?

ptomato commented 2 years ago

I think the question has been answered, so closing.