Unify overloads and documentation for formatValue#5923
Unify overloads and documentation for formatValue#5923dlang-bot merged 4 commits intodlang:masterfrom
Conversation
|
Thanks for your pull request, @JackStouffer! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
Ha! I was doing the same today, but I didn't come around submitting it yet - you bet me to it.
I would leave the full documentation for the individual overloads, but apart from this it already looks good!
std/format.d
Outdated
| } | ||
|
|
||
| /* | ||
| Bools |
There was a problem hiding this comment.
I would leave the full comment line here, e. g.
bool
s are formatted as"true"or"false"with%sand as1or0` with integral-specific format specs.
In this case
JackStouffer
force-pushed
the
formatValue
branch
from
d2f502e to
e1e805a
Compare
|
@wilzbach Done |
JackStouffer
force-pushed
the
formatValue
branch
from
e1e805a to
1050089
Compare
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
Alrighty. LGTM on a glance from my phone. I will go line by line over it later or tomorrow from my notebook and merge this if no one objects until then.
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
I found a few nits, but they aren't related to your precise work, but I found them in the existing code. I will append a commit for these nits to your PR, so if you don't agree you can always drop it.
std/format.d
Outdated
| * | ||
| * Params: | ||
| * w = The $(REF_ALTTEXT output _range, isOutputRange, std,_range,primitives) to write to. | ||
| * obj = The value to write. |
There was a problem hiding this comment.
Nit: some formatValue implementation even call this val and as this function is called formatVALue I think val would be a better fit for the name of this parameter.
The reason why some implementations called it obj is that they then can define sth. like BooleanTypeOf!T val = obj; in their first line and use val consistently.
std/format.d
Outdated
| * | ||
| * Specializations: | ||
| * $(UL $(LI $(D void[]) is formatted like $(D ubyte[]).) | ||
| * $(LI Const array is converted to input range by removing its qualifier.)) |
There was a problem hiding this comment.
Nit: typical formatting of this is:
$(UL
$(LI $(D void[]) is formatted like $(D ubyte[]).)
$(LI Const array is converted to input range by removing its qualifier.)
)
as then it's a lot easier to keep track of the parentheses manually.
(I'm aware that you just copied it 1:1 from the function declaration.)
std/format.d
Outdated
| */ | ||
| @system unittest | ||
| { | ||
| import std.format; |
There was a problem hiding this comment.
This is superfluous - you are already within std.format
(I am aware that this is an old relict)
| @system int delegate(short) @nogc bar() nothrow pure | ||
| { | ||
| int* p = new int; | ||
| return &foo; |
There was a problem hiding this comment.
The function body seems unused. It could be dropped - this will then also increase the coverage and remove the red bits.
3 participants
In the vein of #5191 and #4208, this hides all of the individual overloads of
formatValueand brings the documentation and examples into one place.This is especially useful in the ddox version of the docs, as every overload will no longer be shown on a different page.