[Development] Introducing discussion about QStringFormatter

[Mårten Nordheim]

On 11.08.2017 04:20, Thiago Macieira wrote:
> On quinta-feira, 10 de agosto de 2017 17:53:39 PDT Sze Howe Koh wrote:
>> On 10 August 2017 at 20:41, Mårten Nordheim <marten.nordheim at qt.io> wrote:
>> IMHO, "QFormat" isn't a suitable name. First, this class itself does
>> not describe a format, unlike:
> [cut]
>> For these reasons, I'd personally prefer sticking to "QStringFormatter".
> 
> Agreed, please use QStringFormatter.
> 
>> Having said that, the verbosity of a name is a valid concern. It is
>> the reason why I personally prefer to write raw C string literals
>> instead of wrapping them in QStringLiteral, unless the performance
>> penalty is noticeable. I can't think of a better name than
>> "QStringFormatter" though. Perhaps, in our documentation/examples, we
>> can suggest that the user introduce a typedef in their own code (as
>> opposed to adding an official abbreviation) to shorten things?
>>
>>      typedef QStringFormatter QSF;
>>      QSF formatter(...);
> 
> I disagree here. I don't find it convincing either on typing or on reading.
> First, we don't need to save on keystrokes: you can type "QSFo" and press Ctrl
> +Space on Creator and it'll probably complete to the right class name (if you
> use an IDE that doesn't have this capability, file a suggestion, it's very
> handy; if you don't use an IDE, well, that's your own fault/choice).
> 
> As for the reading, I find that the name conveys just enough information
> without being too verbose.



>>> - ::multiArg was introduced
>>>    - Variadic template which simply forwards each parameter to a
>>>      separate ::arg-call
> 
> Ok, could probably be improved, but no problem right now.
> 
>>>    
>>>    - Currently returns a QString directly. Should it return
>>>      QStringFormatter& like ::arg?
> 
> Yes. Please let all functions that perform replacement return the same thing.
> 
>>> - Static function ::format
>>>
>>>    - Another variadic template, instantiates QStringFormatter and
>>>      forwards arguments to multiArg
>>>      
>>>      - example:
>>>        `QStringFormatter::format("{} {}", "Hello,", "World!");`
> 
> Where are the colons?

It was just a short example, it has the same features as the rest of the 
::args functions:

`QStringFormatter::format("{:<10} {:>10}", "Hello", "World!");`

Though I suppose it should be noted that both ::multiArg (and thus 
::format) currently don't accept any QStringFormatter::Formatting 
arguments, although that overload can be added as well.

>>>      - Remove? Nice to have?
> 
> No, keep. We have a lot of code doing:
> 
> 	QString::fromLatin1("Something %1 $2").arg(arg).arg(arg2);
> 
> might as well keep it easy.
> 
>>> - (QStringFormatter::)Formatting
>>>    - ::arg methods have an optional Formatting argument which can be
>>>      used to override or augment any previously specified in-string
>>>      formatting
>>>    
>>>    - Can be constructed from a string of formatting options
>>>      (e.g. "L<10") or using its methods (e.g. setLocale,
>>>      setJustification)
> 
> Are you saying that the function takes the shorthand-type formatter, instead
> of a more expressive set of information? I understand it makes easy for you,
> since it's the same code anyway, but it sounds contrived.
> 
> But ok, maybe just takes some getting used to.

Do you mean constructors like:
`Formatting(Justify type, int justifyAmount, char fillChar, <rest of 
options>)` ?

It currently lets you write:
`QStringFormatter::Formatting fmt;
fmt.setJustification(QStringFormatter::Left, 10, '-')`

With similar function for all the other formatting options.

>>> - Named replacements uses an alias for QPair<QString, T> called
>>>    Argument.
>>>    - e.g. `QStringFormatter("Hello, {Name}").arg({"Name", "World"});`
> 
> Please pay attention to Marc's post about when to use QPair, std::pair,
> std::tuple in APIs: https://www.kdab.com/tuple-pair-cpp-apis/

Thanks for the link. I agree with the points made in there, but it has 
never been my intention that developers would use Argument themselves 
for anything other than passing it directly to ::arg. Although rewriting 
Argument as a struct is little work.

> If this is just an initializer_list, fair enough, but we need to look into it.
> 
>>>    - A qMakePair-style function called `qArg` for use in
>>>      situations with template argument deduction trouble.
>>>      - e.g.
>>>
>>> `QStringFormatter("Hello, {Name}").arg(QStringFormatter::qArg("Name",
>>> 47));`
> 
> Like this. That looks mighty ugly.

It does. And I'm not sure how to do this part differently. Even if I 
rewrote Argument as a struct rather than a typedef for QPair I would 
still need qArg for when the class can't deduct the template arguments.
Or people would have to write
`QStringFormatter::Argument<int>("Name", 47)`
Which is what I hoped to avoid with "::qArg"

>>> Replacement format
>>> -----
>>>
>>> The replacement options now have formatting features matching
>>> QString::arg. The current options (open to change) are:
>>>
>>> - `L` for localization (localize numbers, like in QString::arg)
>>> - `</>/=` for justification. Left, right and center justification
>>>
>>>    - Followed by an optional padding character (excluding 1-9)
>>>    - Followed by field-width
>>>    - e.g. "==16" (pad using '=', centered, field-width 16),
>>>    
>>>      "<10" (left-justify using spaces, field-width 10),
>>>      ">-3" (right-justify using '-', field-width 3)
> 
> Is this inspired by any API? The one I can think of (printf) uses - for left
> justification and + for right justification. That would make just as much sense
> in using = for center.

Yes:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0645r0.html
https://docs.python.org/3.6/library/string.html#format-specification-mini-language

Although I notice now that they use '^' for centering, and not '='. I 
should probably change to match that.

>>> - `b/B` for setting base. Supports bases 2-36. Using 'b' produces
>>>    lower-case letters for digits 10-35, and 'B' produces upper-case.
>>>    For bases 2-10 they make no difference.
> 
> With shorthands for hex and octal? Can you also make it so that a missing base
> number implies base 2?

No shorthands atm. but I can add those in.
'x'/'X' for hex, and 'o' for octal.

>>> - `:` everything after the colon gets put into an 'extra' field, more
>>>    on this later..
>>>    - e.g. `{:<10:this is the extra field}`
>>>    - or `{::yyyy-MM-dd HH:mm:ss}`
>>>
>>> Currently the formatting options can come in any order (e.g. "L<10"
>>> and "<10L" does the same, the only exception being ':').
>>> However it would be good to enforce some sort of order for the sake
>>> of consistency. With a defined order we could also change
>>> justification format from [<>=]cn to c[<>=]n, which would allow
>>> people to use 1-9 as a fill-character. If this is done, what should
>>> the order be like?
> 
> Agreed on requiring an order, at least in the beginning. What that should be,
> I don't know.
> 
>>> QString::arg compatibility
>>> -----
>>>
>>> Currently QString::arg compatibility is activated using a parameter
>>> in the constructor. All this does is let you use %nn style tokens and
>>> 'disable' use of the brace-style tokens. It also supports `%L` to
>>> enable localization of numbers, but any other formatting must be done
>>> using the `QStringFormatter::Formatting` class.
> 
> Can you already fully replace the QString::arg functions with
> QStringFormatter?

Unless there's something I've missed; yes, all of ::arg's formatting 
functions are implemented.

>>> API for formatting custom types
>>> -----
>>>
>>> One idea I've been experimenting with a little bit is using template
>>> specialization to deal with formatting custom types. To support a
>>> custom type you'd create a specialization of a struct called
>>> `Formatter`. It would use the `extra` field in `Formatting` which
>>> you could optionally parse for custom formatting options. The parser
>>> would be a separate class inheriting `Formatting` and be specified in
>>> the Formatter using a typedef.
>>>
>>> E.g.
>>> `struct QStringFormatter::Formatter<QDateTime>
>>> {
>>> typedef DateTimeInfo FormatType;
>>> QString operator()(const QDateTime &dateTime, const FormatType &format);
>>> }`
>>>
>>> `QStringFormatter` will then instantiate this struct when it receives
>>> a `QDateTime` object, and create a `FormatType` object to parse the
>>> data located in the `extra` field of formatting options. The
>>> `FormatType` object is then expected to store whatever info it needs
>>> and then the `Formatter` will use it later.
>>>
>>> Feedback on the approach, pitfalls etc. is much appreciated!
> 
> Very interesting! I'm just confused why you need two classes here, but I'll
> look into the details when I look at the implementation.

It was in case the developer didn't want/need extra formatting 
information for their type. Then they can write
`typedef QStringFormatter::Formatting FormatType;`
Formatting will then ignore the extra field and will later get passed to 
the class.

>>> Thanks,
>>>
>>>
>>> -- Mårten Nordheim
> 
> Good work!
>