tomaszkam / date

A date and time library based on the C++11/14/17 <chrono> header
Other
0 stars 0 forks source link

[LWG3262] Formatting of negative durations is not specified #23

Closed tomaszkam closed 5 years ago

tomaszkam commented 5 years ago

Original comment:

insert_negative

tomaszkam commented 5 years ago

No idea what to do with this one.

tomaszkam commented 5 years ago

@HowardHinnant: Do you have any idea what this entry on the to-do list was referring to? Tried to search reflector have nothing there. Maybe that something that you received privately?

HowardHinnant commented 5 years ago

Ah, yes. Sorry it is so cryptic. There's a variable in date.h called insert_negative, and it allows for this code to work:

#include "date/date.h"
#include <iostream>

int
main()
{
    using namespace std::chrono;
    std::cout << date::format("%H:%M:%S\n", -10'000s);
}

Output:

-02:46:40

And I don't think the spec currently gives this result. What's worse is that I'm not sure how to modify the spec to say it should work. The trick is is that when working with a negative duration, exactly one of %H, %M or %S should output the negative sign. If all of them did, it looks terrible, and if none of them do, you get the wrong result. And it has to be the first one in the format string to output the negative because the user is going to be using these flags in "big endian" order.

This should work too:

std::cout << date::format("%M:%S\n", -10'000s);

Output

-46:40
tomaszkam commented 5 years ago

@HowardHinnant How about adding new paragraph in [time.format]:

If multiple conversion specifiers are defined for the std::chrono::duration instance holding a negative value, or hh_mm_ss object h for which h.is_negative() is true, the output is equivalent to the output of the corresponding non-negative value, with the - sign applied to the replacement of the leftmost conversion specifier. [ Example: using namespace std::chrono_literals; std::cout << std::format("%H:%M:%S", -10'000s); // prints: -02:46:40 std::cout << std::format("minutes %M, hours %H, seconds %S", -10'000s); // prints: minutes -46, hours 02, seconds 40 -- end example. ]

Of course English needs to polished, suggestion welcomed.

HowardHinnant commented 5 years ago

I think that is very good!

I suggest dropping the first "the" in the first sentence, and placing this paragraph between p16 and p17.

tomaszkam commented 5 years ago

Thanks. I will submit issue once we will get new working draft, as formatting papers applied some changes here.

tomaszkam commented 5 years ago

Remember to simplify specification for hh_mm_ss::operator<<.

tomaszkam commented 5 years ago

I think that these new wording needs also cover the %T case to include sign before it, so the multiple conversion specifier is to restrictive. Maybe:

The result of formatting of std::chrono::duration instance holding a negative value or hh_mm_ss object h for which h.is_negative() is true, is equivalent to the output of the corresponding non-negative value, with the - sign applied to the replacement of the leftmost conversion specifier. [ Example: using namespace std::chrono_literals; std::cout << std::format("%T", -10'000s); // prints: -02:46:40 std::cout << std::format("%H:%M:%S", -10'000s); // prints: -02:46:40 std::cout << std::format("minutes %M, hours %H, seconds %S", -10'000s); // prints: minutes -46, hours 02, seconds 40 -- end example. ]

@HowardHinnant: What do you think?

HowardHinnant commented 5 years ago

The words you quote look good to me.

tomaszkam commented 5 years ago

Discussion: The current specification of the formating for std::chrono::duration and std::hh_mm_ss types is unclear in regards of handling of the negative values. To illustrate:

std::cout << std::format("%H:%M:%S", -10'000s);
// prints: -02:46:40 or -02:-46:-40

The indented (and currently implemented, see https://wandbox.org/permlink/mrw03IaAldP7Bdx1) behavior is to apply the sign once, before the leftmost converted field.

Drafting note: With the above clarification, the specification of the operator<< for hh_mm_ss may be simplified to format("{:%T}", hms).

Proposed resolution: Add the following paragraph after [time.format] Formatting p2: The result of formatting of std::chrono::duration instance holding a negative value, or hh_mm_ss h for which h.is_negative() is true, is equivalent to the output of the corresponding positive value, with the - placed before the replacement of the leftmost conversion specifier. [ Example: using namespace std::chrono_literals; std::cout << std::format("%T", -10'000s); // prints: -02:46:40 std::cout << std::format("%H:%M:%S", -10'000s); // prints: -02:46:40 std::cout << std::format("minutes %M, hours %H, seconds %S", -10'000s); // prints: minutes -46, hours 02, seconds 40 -- end example. ]

Apply following changes to [time.hms.nonmembers] Non-members:

template<class charT, class traits, class Duration> basic_ostream<charT, traits>& operator<<(basic_ostream<charT, traits>& os, const hh_mm_ss& hms);

Effects: Equivalent to: return os << format(os.getloc(), hms.is_negative() ? STATICALLY-WIDEN("-{:%T}") : STATICALLY-WIDEN("{:%T}"), abs(hms.to_duration()));