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

Unexpected behaviour of a format string with alignment and zero #3788

Closed
WolleTD opened this issue Jan 4, 2024 · 2 comments · Fixed by #3789
Closed

Unexpected behaviour of a format string with alignment and zero #3788

WolleTD opened this issue Jan 4, 2024 · 2 comments · Fixed by #3789

Comments

@WolleTD
Copy link
Contributor

WolleTD commented Jan 4, 2024

Hi, I just tried to update fmtlib in a project and one test failed because the output of fmt::format changed for format strings like {:>06x}.

As soon as I add an alignment specifier, the 0 is ignored and the output filled with whitespace instead.
See the comparison between some versions of fmt here: https://godbolt.org/z/KfGjz9TG4

As I understand it, the combination of < and 0 doesn't make much sense, so it's ignored. For symmetry, it's also ignored when any other alignment specifier is present.

I'd say this is desired behaviour, but I miss documentation about it. Probably a note could be added to the paragraph about the '0' specifier?

Preceding the width field by a zero ('0') character enables sign-aware zero-padding for numeric types. It forces the padding to be placed after the sign or base (if any) but before the digits. This is used for printing fields in the form ‘+000000120’. This option is only valid for numeric types and it has no effect on formatting of infinity and NaN. This option is ignored when any alignment specifier is present.

@vitaut
Copy link
Contributor

vitaut commented Jan 4, 2024

Good idea, could you submit a PR to update the doc in https://github.com/fmtlib/fmt/blob/master/doc/syntax.rst?

@WolleTD
Copy link
Contributor Author

WolleTD commented Jan 4, 2024

Of course. Just wanted to look up the commit introducing the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants