Re: [PATCH 3/3] builtin/grep: allow implicit --no-index

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

 



Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:

>> > +#define GREP_NO_INDEX_EXPLICIT 1
>> > +#define GREP_NO_INDEX_IMPLICIT 2
>>
>> I am not sure this is the best way to do this.  For things like
>> this, the usual pattern is to initialize "no_index" to an "unknown"
>> value, allow "--no-index" to toggle it to true (by the way, I think
>> we should reject "--no-no-index", but that is a separate topic), and
>> then after command line parsing finishes, tweak the no_index if it
>> is still "unknown".
>
> The reason for this (and the change in 02/03) is so we can distinguish
> whether there is an explicit no-index or not for the error messages.
> I think it would be okay to have more generic error messages
> ("--cached or --untracked cannot be used without index" and
> "--untracked or no index mode cannot be used with revs").  What do you
> think?

I can understand that you need three states (--no-index given
explicitly from the command line, we fall back to --no-index when we
found we are in a diretory not under control by Git, and we do want
to use the index) to be able to give different messages between the
first two cases.

The usual way we do that is by making the variable tristate (which I
outlined in the previous message, initialize use_index to -1
"unspecified" and then fix it up when it is left unspecified after
you check the config and the command line option).

I however fail to see why that necessitates to change use_index to
no_index, making the code harder to follow by introducing double
negation.

--
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]