#651: Create ISO 8601 Date/Time Values w/o Milliseconds #748
Conversation
|
|
nbcarey
changed the title
| * @example DateTime.utc().set({ hour: 7, minute: 34 }).toISOTime() //=> '07:34:19.361Z' | ||
| * @example DateTime.utc().set({ hour: 7, minute: 34, seconds: 0, milliseconds: 0 }).toISOTime({ suppressSeconds: true }) //=> '07:34Z' | ||
| * @example DateTime.utc().set({ hour: 7, minute: 34 }).toISOTime({ format: 'basic' }) //=> '073419.361Z' | ||
| * @example DateTime.utc().toISOTime({ precision: 'minute' }) //=> '12:34Z' |
There was a problem hiding this comment.
Maybe add a .set({ hour: 7, minute: 34, second: 56 }) for consistency and to show that no rounding is performed
| @@ -1559,22 +1604,26 @@ export default class DateTime { | |||
| * @param {boolean} [opts.suppressSeconds=false] - exclude seconds from the format if they're 0 | |||
| * @param {boolean} [opts.includeOffset=true] - include the offset, such as 'Z' or '-04:00' | |||
| * @param {string} [opts.format='extended'] - choose between the basic and extended format | |||
| * @param {string} [opts.precision='milliseconds'] - specify desired time precision: hours, minutes, seconds or milliseconds | |||
There was a problem hiding this comment.
Add quotes around the possible values to outline strings, like above
| test("DateTime#toISOTime({precision}) throws when the precision is invalid", () => { | ||
| expect(() => dt.toISOTime({ precision: "ms" })).toThrow(InvalidUnitError); | ||
| expect(() => dt.toISOTime({ precision: "xxx" })).toThrow(InvalidUnitError); | ||
| expect(() => dt.toISOTime({ precision: null })).toThrow(InvalidUnitError); |
There was a problem hiding this comment.
Maybe add days, months and years to the list
| expect(dt.toISOTime({ precision: "minute" })).toBe("09:23Z"); | ||
| expect(dt.toISOTime({ precision: "hours" })).toBe("09Z"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Add a test using both precision and suppress[milli]seconds
| // Note that suppressSeconds'/'suppressMilliseconds' take precedence over | ||
| // 'precision'. If you ask for millisecond precison, but have | ||
| // 'suppressSeconds' set to true, You'll get back a string with only | ||
| // hours and minutes when seconds and milliseconds are both zero. |
There was a problem hiding this comment.
I think precision should take precedence instead. It feels more constraining, and suppress* is more cosmetic.
Yes, it would mean that in { precision : 'minutes', suppressSeconds: true } the suppress part is useless, but that feels more intuitive than to display seconds when explicitly asking for a minutes:
12:34:56.789 { precision : 'minutes', suppressSeconds: true } -> 12:34
feels more natural than
12:34:56.789 { precision : 'minutes', suppressSeconds: true } -> 12:34:56
However,
12:34:00 { precision : 'seconds' } -> 12:34:00
12:34:00 { precision : 'seconds', suppressSeconds: true } -> 12:34
There was a problem hiding this comment.
I'll try to get to that this weekend, along with adding days, months and years.
There was a problem hiding this comment.
I agree with the above, but it also reminds me that supressSeconds was never a great name. suppressSecondsIfTheyAreZero is too long. Any ideas?
There was a problem hiding this comment.
hideZeroSeconds / suppressZeroSeconds ?
There was a problem hiding this comment.
hideZeroSeconds sounds good to me
|
Hi, developers. Will this useful pull be merged? Maybe
|
|
Still missing that feature.. |
|
To anyone finding this: this PR just needs a little work, as detailed in the comments. If you do that work, I'll merge it! In the meantime, asking for it more isn't going to help. |
| fmt += ".SSS"; | ||
| } | ||
| // validate the precision and throw if invalid | ||
| let desiredPrecision = Precision[Duration.normalizeUnit(precision)]; |
There was a problem hiding this comment.
Wondering if this needs to call the static normalizeUnit on Duration vs local to datetime.js?
This addresses Issue #651 (Feature request - function to quickly create ISO8601 stamp without milliseconds).
It adds a
precisionoption, the domain of which ishours,minutes,seconds,milliseconds, defaulting tomilliseconds. Time components of higher precision than that specified are simply omitted: no attempt is made at rounding.If
precisionis specified in addition tosuppressSecondsorsuppressMilliseconds, thesuppress*option wins:returns
2020-07-25T12:34Z.Millisecond precision was requested, with the proviso that seconds and milliseconds won't be rendered if they are both zero.
Note: It might be a nice TO DO to add an
autoprecision that would set the precision level to omit trailing components whose value is zero, sowould return
12:34Z.