Re: [PATCH] Add two grep config options

[Michael J Gruber <git@drmicha.warpmail.net>]

Jeff King venit, vidit, dixit 28.03.2011 13:54:
> On Mon, Mar 28, 2011 at 09:24:26AM +0200, Michael J Gruber wrote:
> 
>>>  - This will break scripts people have written, knowing that they can rely
>>>    on "grep" they wrote without giving "-E" from their command line will
>>>    use BRE, and force them to update the script with --no-extended-regexp
>>>    for no good reason.  Worse yet, there isn't even --no-line-numbers
>>>    supported to defeat grep.linenumbers configuration to protect such
>>>    scripts.
>>>
>>>    I understand that some people would feel that the convenience would
>>>    outweigh the risk of script breakage in this particular case, and I am
>>>    sympathetic to the cause, but I still have to point it out.  Is there
>>>    anything we can do to mitigate the risk somehow?
>>
>> This comes up again and again, and I feel that rather than adding config
>> options one by one, we should either allow aliases for standard commands
>> and/or setting default options depending on the mode (ui use vs.
>> scripting use), so to say a companion to "git -c n=v" which allows
>>
>> git config ui.grep "-E -n"
>>
>> I.e. just like "git -c n=v <cmd>" sets up pseudo config before running
>> cmd, our wrapper could augment argv from "ui.<cmd>".
>>
>> We could safeguard scripts from this by
>>
>> - checking istty and
>> - checking env for GIT_PLUMBING
>>
>> and setting the latter in git-sh-setup.sh. After a long migration phase,
>> we could skip the first (fragile) check.
> 
> I like the idea of a GIT_PLUMBING variable. It is something that has
> come up as a solution to similar problems many times in the past, but it
> never ends up getting implemented, because it never solves the problem
> at hand. It is something that we would introduce now, and would solve
> problems several versions down the road, after people adjusted their
> scripts. So nobody ends up doing it.
> 
> And probably we would want something like --no-plumbing to switch back
> to porcelain mode for a specific command.  Because often scripts do a
> bunch of work, and then want to show the user their output in their
> favorite format.  Something like:
> 
>   . git-sh-setup ;# or manually GIT_PLUMBING=1
>   # get list of "interesting" files containing some pattern
>   files=`git grep -le "$1"`
>   # and then show them
>   git --no-plumbing log $files
> 
> One shortcoming of such a scheme, though, is that it is an
> all-or-nothing proposal; a script has no way to say "it's OK to take the
> user's default for _this_ option, but not for others". For example, in
> the script above, it would make sense for the grep call to respect the
> user's choice of "-E". So it is tempting to use --no-plumbing there,
> too. But we don't necessarily want to respect whatever cruft the user
> put into ui.grep, because we care what the output looks like (something
> like "-O" would not be helpful).
> 
> So what we really want is to let the script "allow" certain options from
> the user's preferences. This could be done easily with individual config
> options, like:
> 
>   git --allow=grep.extended grep ...
> 
> where the config code would respect a variable if GIT_PLUMBING is unset,
> or if the key is in the allowed list. You could probably also adapt it
> to something like:
> 
>   git --allow="-E --foo" grep ...
> 
> which would allow "-E" and "--foo" in ui.grep, but nothing else.
> 
> Now, obviously my script is a toy (it's not a real script I use). In
> particular, "-O" would probably be suppressed by the lack of a tty,
> anyway. But:
> 
>   1. I used a toy to have something readable. Look at something more
>      complex, like "git pull". It calls "git merge". Should it be
>      respecting "-n" and "--log" from the user's config? Probably.
>      Should it respect "--no-ff" or "-s"? I'm not sure. And there are
>      lots more examples just in the git.git scripts.
> 
>   2. It's not just about safeguarding versus the current set of grep
>      options. It's about safeguarding the script against any _future_
>      options that don't exist yet.
> 
>   3. This might be overengineering a little bit. Most invocations
>      probably do fall into either the "run command to get its output"
>      category (where you want pure plumbing) or the "run command to show
>      things to the user" category (where you will take whatever options
>      the user wants). But because the deployment of whatever scheme is
>      chosen is going to be so annoying (because it will take many
>      versions before we're clear to assume that people are setting
>      GIT_PLUMBING), I would rather over-engineer than find out 2 years
>      into the process that the flexibility we provide is not sufficient
>      and have to start again.
> 
>> We could safeguard scripts from this by
>>
>> - checking istty and
>> - checking env for GIT_PLUMBING
> 
> I'm not sure isatty is a good check. In the example above, grep's output
> was not going to a tty, but I did want to respect the user's choice of
> "-E".

I'm not saying it's good either, but it is something that a new git
(i.e. between the time we introduce ui.* and GIT_PLUMBING/--no-plum and
the time we rely on the latter) could do to make use of (and promote)
the new ui.* options.

Michael