Re: [PATCH] grep: allow -E and -n to be turned on by default via configuration
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 28, 2011, 23:16 UTC
- Message-ID
- <7vr59qvnt3.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <AANLkTimpp+dyt8cONp_nK72eTXXgsOfkHuWpPTr90h=u@mail.gmail.com>
Joe Ratterman <jratt0@gmail.com> writes:
Show 11 quoted lines
> On Mon, Mar 28, 2011 at 5:12 PM, Junio C Hamano <gitster@pobox.com> 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.
Show 5 quoted lines
>> + 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.