Joe Ratterman <jratt0@xxxxxxxxx> writes:
> On Mon, Mar 28, 2011 at 5:12 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> +grep.lineNumbers::
>> + Â Â Â If set to true, enable '-n' option by default.
>> +
>> +grep.extendedRegexp::
>> + Â Â Â If set to true, enable '--extended-regexp' option by default.
>> +
>
> I know my original patch was plural, but I since noticed that the GNU
> grep --line-number option is singular. I used the same thing in my
> patch to add that option to git grep. Should this one be singular?
Good eyes.
We should match what we are trying to mimic, and we need to be internally
consistent. So --[no-]line-number should be singular to mimic GNU (which
is how your othr patch is done---I don't have to amend what I already
queued). And we should match the variable by naming it "grep.lineNumber"
to the command line option.
>> + Â Â Â if (!strcmp(var, "grep.linenumbers")) {
>> + Â Â Â Â Â Â Â opt->linenum = git_config_bool(var, value);
>
> We need to match the case between the docs and the code or use
> strcasecmp().
In git_config() callback functions, the variable name is supposed to be
already downcased by the caller, except for the second-level name in
three-level names, like "branch.Foo.merge" where the second-level name is
case sensitive. Feeding var we got from the caller and lowercased
variable name we expect to strcmp() is the right way to write them when
you are dealing with configuration variable names.
Some places might unnecessarily and incorrectly use strcasecmp() but if
so, we should be fixing them instead. We shouldn't mimic bad code.
In documentation we customary write them in camelCase only for
readability.
--
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