From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 26 Mar 2010 16:18:06 GMT
Subject: Re: [PATCH 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]
Message-ID: <alpine.DEB.1.00.1003261714550.7596@pacific.mpi-cbg.de>
In-Reply-To: <36ca99e91003260439x3d60b851g15b8652d50f61c06@mail.gmail.com>

Hi,

On Fri, 26 Mar 2010, Bert Wesarg wrote:

> On Fri, Mar 26, 2010 at 11:48, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > This supports opening the results of a 'git grep' directly in a pager
> > (where the pager can be 'vi', too).
> >
> > This series is purely about convenience, everything the option does can
> > be done with a regular script or command line.
> >
> > But I saw so many people doing their own scripts for that, and in many
> > cases, they are subtly broken (e.g.
> >
> >        git grep -z <expr> | xargs -0r vi +/<expr>
> >
> > would work as long as you do not have to check the exit status of git
> > grep from another script) that I finally decided to go for it and send
> > this patch pair.
> >
> > My most common use case for this is to do something like
> >
> >        git grep -Ovi SomeJustRenamedFile
> >
> > to edit all files I might have forgotten to change after a git mv.
> >
> > (Actually, to be honest, my use case involves -Pvi, but I will have to
> > retrain my hands.)
> >
> > Johannes Schindelin (2):
> >  grep: Add the option '--open-files-in-pager'
> >  grep -P: allow optional argument specifying the pager (or editor)
> 
> Your re-training failed miserable ;-)

It would have if I said that it is finished.

> I like the idea. But could it make sense to get the line number of the 
> first match to the filename. In the same syntax as on the console (i.e. 
> <file>:<line>:)?

See Peff's mail, and my answer to it for an explanation why it would 
actively _not_ help me that way.

> I have also the feeling that -O potion does not like pager with
> arguments, be it from GIT_PAGER, PAGER, or from the command line.

You are correct. That's why I said "-O[<pager>]" and not 
"-O[<pager-with-arguments-that-cause-whitespace-problems-especially-with-spaces-in-directory-names-so-you-need-to-make-a-script-wrapper-anyway>]" 

Of course, if you would find that useful, nobody stops you from making a 
patch on top of my patch pair.

Ciao,
Dscho