Re: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 30, 2015, 16:17 UTC
Message-ID: <xmqqsi857h8d.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CA+P7+xpYg4wsJW8JOGHMsDohCUvcD=X-34wcGGvEQM8mEK3dbg@mail.gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

Show 22 quoted lines

> On Wed, Jul 29, 2015 at 2:30 PM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>>
>>> Also, please explain here and in the commit message why this highly
>>> specialized colorizer ('colornext'), is needed even though a more
>>> general purpose one ('color') is already available.
>>
>> It is needed in the current form to allow
>> %(colornext:blue)%(ifexists:[%s]) to color only the replacement of %s
>> and not the [].
>>
>> But I now think that this would be more elegantly solved by Junio's
>> %(if) %(endif) idea:
>>
>>   %(if:atom) [ %(color:blue)%(atom)%(color:reset) ] %(endif)
>>
>> (I added spaces for clarity)
>
> I agree, this style seems a lot more elegant and expressive while much
> easier to understand. Same for doing this to the alignment atoms and
> such as it solves the same problem(s) there.

Do you mean something like these?

    %(align:left,20) branch %(refname:short) %(end)
    %(align:middle,20) branch %(refname:short) %(end)
    %(align:right,20) branch %(refname:short) %(end)

to replace and enhance %(padright:20)?

> I can't speak to how easy it is to implement tho.

Perhaps it would go like this:

 * Instead of always emitting to the standard output, emit() and
   print_value() will gain an option to append into a strbuf that is
   passed as argument.  Alternatively, appending to strbuf could be
   made the only output channel for them; show_ref_array_item() can
   prepare an empty strbuf, call them repeatedly to fill it, and
   then print the resulting strbuf itself.

 * Things like %(if) and %(align) would do the following:

   (1) Push the currently active strbuf it got from the calling
       show_ref_array_item() to the formatting state;

   (2) Create a new strbuf and arrange so that further output would
       be diverted to this new one; and

   (3) Push the fact that the diverted output will be processed by
       them (i.e. %(if), %(align), etc.) when the diversion finishes
       to the formatting state.

 * When %(end) is seen, the currently active strbuf (i.e. the new
   one created in (2) above for diversion) holds the output made
   since the previously seen %(if), %(align), etc.  The formatting
   state knows what processing needs to be performed on that from
   (3) above.

   - Pop the strbuf where the output of the entire %(if)...%(end)
     construct needs to go from the formatting state;

   - Have the processing popped from (3) above, e.g. %(if:atom) or
     %(align:left,20), do whatever they need to do on the diverted
     output, and emit their result.

Both %(if) and the hypotherical %(align) can use this same "diversion" mechanism. And the above would properly nest, e.g.

    %(align:middle,40)%(if:taggerdate)tag %(end)%(refname:short)%(end)