Re: [PATCH v4 7/7] contrib/git-jump/git-jump: jump to match column in addition to line

[Taylor Blau]

On Sun, May 06, 2018 at 08:03:01PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Sun, May 06 2018, Martin Ågren wrote:
>
> > On 5 May 2018 at 04:43, Taylor Blau  wrote:
> >> Take advantage of 'git-grep(1)''s new option, '--column' in order to
> >> teach Peff's 'git-jump' script how to jump to the correct column for any
> >> given match.
> >>
> >> 'git-grep(1)''s output is in the correct format for Vim's jump list, so
> >> no additional cleanup is necessary.
> >
> >> diff --git a/contrib/git-jump/README b/contrib/git-jump/README
> >> index 4484bda410..7630e16854 100644
> >
> >>  # use the silver searcher for git jump grep
> >> -git config jump.grepCmd "ag --column"
> >> +git config jump.grepCmd "ag"
> >
> > I think this change originates from Ævar's comment that it "also makes
> > sense to update the example added in 007d06aa57 [...] which seems to
> > have added jump.grepCmd as a workaround for not having this" [1].
> >
> > Somehow though, this approach seems a bit backwards to me. I do believe
> > that your series reduces the reasons for using `jump.grepCmd`, but
> > regressing this example usage of `jump.grepCmd` seems a bit hostile. If
> > someone wants to use `ag`, wouldn't we want to hint that they will
> > probably want to use `--column`?
> >
> > Is there some other `ag --column --foo` that we can give, where `--foo`
> > is not yet in `git grep`? ;-)
> >
> > Martin
> >
> > [1] https://public-inbox.org/git/874lk2e4he@evledraar.gmail.com/
>
> Yeah it doesn't make sense to drop --column here, FWIW what I had in
> mind was something closer to:

Thanks; I wasn't quite clear on what you had suggested in [1], so the
attached diff is very helpful.

> diff --git a/contrib/git-jump/README b/contrib/git-jump/README
> index 4484bda410..357f79371a 100644
> --- a/contrib/git-jump/README
> +++ b/contrib/git-jump/README
> @@ -25,6 +25,13 @@ git-jump will feed this to the editor:
>  foo.c:2: printf("hello word!\n");
>  ---
>
> +Or, when running 'git jump grep' column numbers will also be emitted,
> +e.g. `git jump grep "hello"' would return:
> +
> +---
> +foo.c:2:10: printf("hello word!\n");
> +---
> +
>  Obviously this trivial case isn't that interesting; you could just open
>  `foo.c` yourself. But when you have many changes scattered across a
>  project, you can use the editor's support to "jump" from point to point.
>
> I.e. let's note what the output format is now like for 'grep', and no
> need to change the jump.grepCmd.

Applied (mostly) the above patch to my copy, and will attach as part of
v5.

> The above patch may be incorrect when it comes to the line numbe /
> column number / format, I just wrote that by hand.

Yes; the only thing that was wrong was the column number. The "w" is in
the 10th 1-indexed column, and 'git grep --column' uses 1-indexed
columns.

Thanks,
Taylor

Re: [PATCH v4 7/7] contrib/git-jump/git-jump: jump to match column in addition to line

[Ævar Arnfjörð Bjarmason]

On Sun, May 06 2018, Martin Ågren wrote:

> On 5 May 2018 at 04:43, Taylor Blau  wrote:
>> Take advantage of 'git-grep(1)''s new option, '--column' in order to
>> teach Peff's 'git-jump' script how to jump to the correct column for any
>> given match.
>>
>> 'git-grep(1)''s output is in the correct format for Vim's jump list, so
>> no additional cleanup is necessary.
>
>> diff --git a/contrib/git-jump/README b/contrib/git-jump/README
>> index 4484bda410..7630e16854 100644
>
>>  # use the silver searcher for git jump grep
>> -git config jump.grepCmd "ag --column"
>> +git config jump.grepCmd "ag"
>
> I think this change originates from Ævar's comment that it "also makes
> sense to update the example added in 007d06aa57 [...] which seems to
> have added jump.grepCmd as a workaround for not having this" [1].
>
> Somehow though, this approach seems a bit backwards to me. I do believe
> that your series reduces the reasons for using `jump.grepCmd`, but
> regressing this example usage of `jump.grepCmd` seems a bit hostile. If
> someone wants to use `ag`, wouldn't we want to hint that they will
> probably want to use `--column`?
>
> Is there some other `ag --column --foo` that we can give, where `--foo`
> is not yet in `git grep`? ;-)
>
> Martin
>
> [1] https://public-inbox.org/git/874lk2e4he@evledraar.gmail.com/

Yeah it doesn't make sense to drop --column here, FWIW what I had in
mind was something closer to:

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 4484bda410..357f79371a 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -25,6 +25,13 @@ git-jump will feed this to the editor:
 foo.c:2: printf("hello word!\n");
 ---

+Or, when running 'git jump grep' column numbers will also be emitted,
+e.g. `git jump grep "hello"' would return:
+
+---
+foo.c:2:10: printf("hello word!\n");
+---
+
 Obviously this trivial case isn't that interesting; you could just open
 `foo.c` yourself. But when you have many changes scattered across a
 project, you can use the editor's support to "jump" from point to point.

I.e. let's note what the output format is now like for 'grep', and no
need to change the jump.grepCmd.

The above patch may be incorrect when it comes to the line numbe /
column number / format, I just wrote that by hand.

Re: [PATCH v4 7/7] contrib/git-jump/git-jump: jump to match column in addition to line

[Martin Ågren]

On 5 May 2018 at 04:43, Taylor Blau  wrote:
> Take advantage of 'git-grep(1)''s new option, '--column' in order to
> teach Peff's 'git-jump' script how to jump to the correct column for any
> given match.
>
> 'git-grep(1)''s output is in the correct format for Vim's jump list, so
> no additional cleanup is necessary.

> diff --git a/contrib/git-jump/README b/contrib/git-jump/README
> index 4484bda410..7630e16854 100644

>  # use the silver searcher for git jump grep
> -git config jump.grepCmd "ag --column"
> +git config jump.grepCmd "ag"

I think this change originates from Ævar's comment that it "also makes
sense to update the example added in 007d06aa57 [...] which seems to
have added jump.grepCmd as a workaround for not having this" [1].

Somehow though, this approach seems a bit backwards to me. I do believe
that your series reduces the reasons for using `jump.grepCmd`, but
regressing this example usage of `jump.grepCmd` seems a bit hostile. If
someone wants to use `ag`, wouldn't we want to hint that they will
probably want to use `--column`?

Is there some other `ag --column --foo` that we can give, where `--foo`
is not yet in `git grep`? ;-)

Martin

[1] https://public-inbox.org/git/874lk2e4he@evledraar.gmail.com/

[PATCH v4 7/7] contrib/git-jump/git-jump: jump to match column in addition to line

[Taylor Blau]

Take advantage of 'git-grep(1)''s new option, '--column' in order to
teach Peff's 'git-jump' script how to jump to the correct column for any
given match.

'git-grep(1)''s output is in the correct format for Vim's jump list, so
no additional cleanup is necessary.

Signed-off-by: Taylor Blau 
---
 contrib/git-jump/README   | 6 +++---
 contrib/git-jump/git-jump | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 4484bda410..7630e16854 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -35,7 +35,7 @@ Git-jump can generate four types of interesting lists:
 
   2. The beginning of any merge conflict markers.
 
-  3. Any grep matches.
+  3. Any grep matches, including the column of the first match on a line.
 
   4. Any whitespace errors detected by `git diff --check`.
 
@@ -65,7 +65,7 @@ git jump grep foo_bar
 git jump grep -i foo_bar
 
 # use the silver searcher for git jump grep
-git config jump.grepCmd "ag --column"
+git config jump.grepCmd "ag"
 --
 
 
@@ -82,7 +82,7 @@ which does something similar to `git jump grep`. However, it 
is limited
 to positioning the cursor to the correct line in only the first file,
 leaving you to locate subsequent hits in that file or other files using
 the editor or pager. By contrast, git-jump provides the editor with a
-complete list of files and line numbers for each match.
+complete list of files, lines, and a column number for each match.
 
 
 Limitations
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 80ab0590bc..931b0fe3a9 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -52,7 +52,7 @@ mode_merge() {
 # editor shows them to us in the status bar.
 mode_grep() {
cmd=$(git config jump.grepCmd)
-   test -n "$cmd" || cmd="git grep -n"
+   test -n "$cmd" || cmd="git grep -n --column"
$cmd "$@" |
perl -pe '
s/[ \t]+/ /g;
-- 
2.17.0