Various issues in fprint.

[orlp]

Various issues in fprint.

[Rapptz]

Thanks. I'll look into this later.

[Rapptz]

I'm getting around to this tonight. I have a few responses though.

Potential issue: format string is totally ignored if no additional arguments were passed. You mentioned this was by design, but I strongly recommend you to reconsider it as it potentially breaks generic code that relies on fprint, and if a user really wants the faster "unformatted formatting" they can always resort to just calling out << str .

This is a legitimate issue. I figure I might as well do it.

Doc issue: format-string is a confusing identifier in your grammar since you have the format string, containing literal text and format-strings. See the issue? I suggest taking over Pythons terminology and calling it a "replacement field".

The entire string is called a format-string. It's divided into | <parameter> : <format-spec> |. I see no ambiguity.

Critical issue: formatting is not stateless, as you do not correctly set default fill parameters, and they can carry over from replacement field to replacement field. For example: https://gist.github.com/orlp/a0aba247345baaf8c393 I only checked this for fill, but the same probably applies to precision, align, width and potentially more.

Valid concern. I'll fix it.

Checking issue: you do not check if an integer came after the dot in ["." precision]. An example invalid format string that you do not throw an exception on is |:.|

I'll fix this too.

Missing feature: std::hexfloat - you said GCC didn't support this, but I list it here for completion anyway.

I see no purpose of supporting this at the moment since I have to compile with GCC 4.8 or higher. I'm not going to drop support for GCC 4.8 and 4.9 just for a single component. I might make a macro to solve this (similar to GEARS_META_CPP14) but it's very low in my priority list and I feel like it might be more confusing than anything.

Missing feature: std::showpoint - I suggest changing p to std::showpoint and assigning + to std::showpos.

Valid concern. I'll fix this.

[Rapptz]

Fixed in a21a017. If you see anything else missing feel free to reopen this issue.

[orlp]

Two things:

1. The entire string is called a format-string. It's divided into | : |. I see no ambiguity.

   Here's the thing, the parameter itself to the fprint function is what you would call a
   "format string". This in turn contains literal characters and replacement fields. In your
   grammar the literal characters are ignored, and you make it seem like there is only one
   replacement field in the entire format string:

   format-string ::= "|" <`parameter`> [":" `format-spec`] "|"
   

   This would be better IMO:

   format-string ::= [replacement-field | <any character>]*
   replacement-field ::= "|" <`parameter`> [":" `format-spec`] "|"
   

2. In stream_state.reset you use the value 0x20. Is out.widen(' ') always guaranteed to be 0x20?

[Rapptz]

I'm not sure if number 1 is a really big issue. I don't explicitly see the confusion but I'll fix it anyway.

With regards to number 2, I got to thinking and I realised the small subset of ASCII (i.e 0x00 - 0x7F) is probably not succumbed to widening. Only those that have a multi-byte representation in different locales do. I think ASCII is safe so 0x20 is guaranteed in at least Windows and Linux. So I might replace all the widen calls that are < 0x7F with explicit checking with a number and probably use eq_int_type instead of eq.