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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 27, 2015 at 7:58 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
>
>> Karthik Nayak <karthik.188@xxxxxxxxx> 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.

>
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]