Re: [PATCH v4 6/6] for-each-ref: avoid color leakage

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

 



Junio C Hamano wrote:
> My knee-jerk "adding it to struct refinfo" is not correct, either,
> because what we want to know, i.e. "do we need an extra reset?" is
> not a property for each ref.  It is similar to "what is the set of
> atoms the format string is using?" and "do we need to peel tags in
> order to show all information asked by the format string?"
> (i.e. used_atom[] and need_tagged, respectively).

It's mostly cruft carried over from my reset-color-after-each-token
implementation. My severe distaste for global variables prevented me
from looking for the simpler solution.

> Unlike need_tagged which asks "is there any *refname that asks us to
> peel tags?", however, "is the _last_ color:anything in the format
> string not 'reset'?" depends on the order of appearance of atoms in
> the format string, so this needs to be done in a loop that scans the
> format string from left to right once at the very beginning, and we
> have a perfect place to do so in verify_format().

I should be shot for my laziness and lack of ingenuity. Yeah,
verify_format() is a much better place to put the logic than
populate_value().

> So perhaps like this one on top?
>
>  builtin/for-each-ref.c | 45 +++++++++++++++++++++------------------------
>  1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 04e35ba..5ff51d1 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c

Thanks for taking the time to do this. And apologies for the laziness.
--
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]