-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Improve sprintf docs, minor String::Formatter refactors #6758
Conversation
src/kernel.cr
Outdated
@@ -73,7 +73,7 @@ end | |||
# The syntax for a format specifier is: | |||
# | |||
# ```text | |||
# %[flags][width][.precision]type | |||
# %[flags][width][.precision][type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What? The type is required. That's why it is not in brackets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I didn't realize the brackets indicate that the argument is optional.
src/string/formatter.cr
Outdated
@@ -110,7 +110,7 @@ struct String::Formatter(A) | |||
|
|||
private def consume_width(flags) | |||
case current_char | |||
when '1'..'9' | |||
when '0'..'9' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is wrong, unless the 0 is after a non-0 digit, 0 is a separate flag:
Line 150 in 3917852
# 0 (zero) | bdiouxX | Pad with zeros, not spaces. |
(I can't fully check the implementation, i'm on my phone)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @r00ster91 👍
This removes redundant spaces in the
sprintf
documentation. The spaces have just been copied from Ruby'ssprintf.c
. They are there because Ruby uses the 4-space-code-markdown but in Crystal that's not required because Crystal uses the ```-markdown.And this also adds the missing
c
format flag. It's documented but not actually implemented.