Re: [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state`
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jul 27, 2015, 17:23 UTC
- Message-ID
- <CAOLa=ZQABLyaKVXub_dzCZem=aK-pFr5odM4aXaKjnMN6Rk=kQ@mail.gmail.com>
- In-Reply-To
- <xmqq4mkpk73h.fsf@gitster.dls.corp.google.com>
On Mon, Jul 27, 2015 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 29 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes: > >> Karthik Nayak <karthik.188@gmail.com> 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.
Yes even I agree with this.
Show 9 quoted lines
> > 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).
This depends on whether these modifier atoms should effect only succeeding atoms or any string. Since there is a conversation about this topic on the other patch, let's continue this discussion there.
> > Thanks for finding this, and Karthik, sorry for an incomplete > suggestion based on a faulty thinking.
I don't see why you have to be sorry, you were trying to help :)
-- Regards, Karthik Nayak