[Development] Introducing discussion about QStringFormatter

[Thiago Macieira]

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?

> >     - 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.

> > - 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/

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.

> > 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.

> > - `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?

> > - `:` 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?

> > 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.

> > Thanks,
> > 
> > 
> > -- Mårten Nordheim

Good work!

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center