[PATCH] YAML: Add an optional 'flow' field to the mapping trait to allow flow mapping output.

[Justin Bogner]

Alex L <arphaman at gmail.com> writes:
> 2015-05-01 17:21 GMT-07:00 Justin Bogner <mail at justinbogner.com>:
>> Alex Lorenz <arphaman at gmail.com> writes:
>>> @@ -427,8 +432,23 @@
>>>                            bool &UseDefault, void *&) {
>>>    UseDefault = false;
>>>    if (Required || !SameAsDefault) {
>>> -    this->newLineCheck();
>>> -    this->paddedKey(Key);
>>> +    auto State = StateStack.back();
>>> +    if (State == inFlowMapFirstKey || State == inFlowMapOtherKey) {
>>> +      if (State == inFlowMapOtherKey)
>>> +        output(", ");
>>> +      if (Column > 70) {
>>> +        output("\n");
>>> +        for (int i = 0; i < ColumnAtMapFlowStart; ++i)
>>> +          output(" ");
>>> +        Column = ColumnAtMapFlowStart;
>>> +        output("  ");
>>> +      }
>>> +      output(Key);
>>> +      output(": ");
>>> +    } else {
>>> +      this->newLineCheck();
>>> +      this->paddedKey(Key);
>>> +    }
>>
>> So to be consistent this should really be split into preflightKey and
>> preflightFlowKey, much like is done for elements. As it is here, there's
>> a big difference in abstraction level between the two branches of the
>> if, which makes this hard to follow.
>
> I can extract the preflightFlowKey into a separate private method
> that's called by preflightKey, but it wouldn't be necessarily
> consistent I think as this won't be another virtual method from IO
> like the others and it would also have no postflightFlowKey sibling
> method.

Fair enough. Do extract this anyway, but I guess call it something like
flowKey() to match paddedKey(). The switch between "operations on
strings" and "calls to high-level helpers" makes this harder to read
than it needs to be.