git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Add two grep config options

From
Michael J Gruber <git@drmicha.warpmail.net>
Date
Mar 28, 2011, 12:00 UTC
Message-ID
<4D90787A.8010403@drmicha.warpmail.net>
In-Reply-To
<20110328115421.GA9232@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 28.03.2011 13:54:
Show 106 quoted lines
> 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
Previous: Jeff KingNext: Jeff King
Message 5 of 19 in “Add two grep config options”
  1. Add two grep config optionsJoe Ratterman, Mar 25, 2011
  2. Junio C HamanoMar 25, 2011
  3. Michael J GruberMar 28, 2011
  4. Jeff KingMar 28, 2011
  5. Michael J GruberMar 28, 2011
  6. Jeff KingMar 28, 2011
  7. Junio C HamanoMar 28, 2011
  8. Jeff KingMar 28, 2011
  9. Junio C HamanoMar 28, 2011
  10. Joe RattermanMar 28, 2011
  11. Bert WesargMar 28, 2011
  12. Joe RattermanMar 28, 2011
  13. Junio C HamanoMar 28, 2011
  14. grep: allow -E and -n to be turned on by default via configurationJunio C Hamano, Mar 28, 2011
  15. Junio C HamanoMar 28, 2011
  16. Joe RattermanMar 28, 2011
  17. Junio C HamanoMar 28, 2011
  18. Joe RattermanMar 29, 2011
  19. grep: allow -E and -n to be turned on by default via configurationJoe Ratterman, Mar 30, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.