Subject: Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`

From: Junio C Hamano <[email-masked]>
Date: Mon, 27 Jul 2015 07:28:34 -0700

Matthieu Moy <[email-masked]> writes:

> Karthik Nayak <[email-masked]> writes:
>
>> Make the `color` atom a pseudo atom and ensure that it uses
>> `ref_formatting_state`.
>
> Actually, I think this is an incorrect change.
>
> Previously, %(color:...) was taking effect immediately, and after this
> patch, it takes effect starting from the next atom.
> ...
> As a consequence of the remark above, I think the name and comment of
> the field are misleading. There are 3 kinds of atoms:
>
> * Placeholders for ref attributes
>
> * Atoms that take action immediately, but that are not ref attributes
>   like %(color)
>
> * Atoms that only act as modifiers for the next atom. Perhaps they could
>   be called "prefix atoms" or "modifier atoms".

My fault.

I briefly thought that it may be simpler to treat %(color) just as a
different way to express a literal that cannot be typed from the
keyboard, but your "different kind of atom" is a much better way to
think about it.

What is necessary is that, just like the updated "print_value()"
knows about the formatting state, "emit()" needs to be told about
the same formatting state.  Some of these "state affecting" atoms
may take effect on what is output by "emit()" (e.g. "color" is
obviously an example, the hypotethical one that counts the current
output column and uses that knowledge to "align" the output to
certain columns, instead of "right pad to make the next column
30-columns wide" one, which is in this series, is another).

Thanks for finding this, and Karthik, sorry for an incomplete
suggestion based on a faulty thinking.