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

Add POSIX's apostrophe flag for printf-based funcs #1285

Merged
merged 1 commit into from
Sep 15, 2024

Conversation

GabrielRavier
Copy link
Collaborator

@GabrielRavier GabrielRavier commented Sep 8, 2024

POSIX specifies the flag character for printf as formatting
decimal conversions with the thousands' grouping characters specified by
the current locale. Given that cosmopolitan currently has no support for
obtaining the locale's grouping character, all that is required (when in
the C/POSIX locale) for supporting this flag is ignoring it, and as it's
already used to indicate quoting (for non-decimal conversions), all that
has to be done is to avoid having it be an alias for the flag so
that decimal conversions don't accidentally behave as though the
flag has also been specified whenever the flag is utilized.

This patch adds this flag, as described above, along with a test for it.

@github-actions github-actions bot added the libc label Sep 8, 2024
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do support grouping. You can say %,d for 1,000,000. Could you please change %'d so it formats 1'000'000? Then afterwards we should probably look into importing Musl's localeconv() function so we can access its grouping field. I want the default behavior for Cosmopolitan Libc to be using that apostrophe since it's culture agnostic and I have a disability that makes me allergic to calling setlocale() and unable to read large numbers without grouping. Obviously though if the locale actually specifies a grouping (e.g. English, German) then we should use what it specifies. If you decide to do this (I will if you won't) then please take care to ensure that obtaining the current locale grouping doesn't link any locale code. It should only need to access a global variable through TLS I think.

@GabrielRavier
Copy link
Collaborator Author

GabrielRavier commented Sep 10, 2024

Well, given that in the C/POSIX locale (the mandatory default before setlocale is ever called) the thousands grouping character is nothing (as specified in the LC_NUMERIC in the POSIX Locale section of the POSIX standard), I'm pretty sure using ' as the separator without being in the context of an appropriate locale is a completely and utterly non-standard implementation of this flag. I guess I'll implement it like that if you want (well, when I find the time to do it, so perhaps not today) but I would just like to say that it is 100% a POSIX violation (btw, I can confirm it was the absence of localeconv() that made me think cosmopolitan didn't handle grouping characters).

@GabrielRavier
Copy link
Collaborator Author

I've added a commit to make this change.

@jart
Copy link
Owner

jart commented Sep 11, 2024

I can see that you view this as something important and the standard is on your side. In that case, I'm convinced. Could you please update this PR to go back to the conformant behavior you originally chose?

@GabrielRavier GabrielRavier force-pushed the feat/printf-apostrophe-flag branch 2 times, most recently from 881d151 to 7b4e85c Compare September 12, 2024 14:29
@GabrielRavier
Copy link
Collaborator Author

I have just done so.

POSIX specifies the <apostrophe> flag character for printf as formatting
decimal conversions with the thousands' grouping characters specified by
the current locale. Given that cosmopolitan currently has no support for
obtaining the locale's grouping character, all that is required (when in
the C/POSIX locale) for supporting this flag is ignoring it, and as it's
already used to indicate quoting (for non-decimal conversions), all that
has to be done is to avoid having it be an alias for the <space> flag so
that decimal conversions don't accidentally behave as though the <space>
flag has also been specified whenever the <apostrophe> flag is utilized.

This patch adds this flag, as described above, along with a test for it.
@jart jart merged commit 675abfa into jart:master Sep 15, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants