[PATCH 0/2] git-jump: support Emacs

[PATCH 0/2] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Yoichi Nakayama (2):
  git-jump: add an optional argument 'stdout'
  git-jump: invoke emacsclient

 contrib/git-jump/README   |  9 ++++++++-
 contrib/git-jump/git-jump | 22 +++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1423
-- 
gitgitgadget

[PATCH 1/2] git-jump: add an optional argument 'stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   |  9 ++++++++-
 contrib/git-jump/git-jump | 11 ++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..6aaa6a928d2 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,13 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument 'stdout' to print the listing to
+standard output. You can use it with M-x grep on Emacs.
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump stdout <mode>"
+Run grep (like this): git jump stdout diff
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +107,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..a907f69304d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `stdout` is given, print the quickfix
+lines to standard output.
 EOF
 }
 
@@ -69,6 +72,12 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+if test "$mode" = "stdout"; then
+	mode=$1; shift
+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+	"mode_$mode" "$@" 2>/dev/null
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

[PATCH 2/2] git-jump: invoke emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacsclient" or GIT_EDITOR="emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index a907f69304d..536f0341aaf 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -26,6 +26,11 @@ open_editor() {
 	eval "$editor -q \$1"
 }
 
+open_emacsclient() {
+	editor=`git var GIT_EDITOR`
+	eval "$editor -e \"(prog1 (switch-to-buffer-other-frame (grep \\\"git jump stdout $@\\\")) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
+}
+
 mode_diff() {
 	git diff --no-prefix --relative "$@" |
 	perl -ne '
@@ -79,6 +84,12 @@ if test "$mode" = "stdout"; then
 	exit 0
 fi
 
+if git var GIT_EDITOR | grep ^emacsclient >/dev/null; then
+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+	open_emacsclient "$mode" "$@"
+	exit 0
+fi
+
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
 type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
-- 
gitgitgadget

Re: [PATCH 2/2] git-jump: invoke emacsclient

[Eric Sunshine]

On Sat, Nov 19, 2022 at 9:09 AM Yoichi Nakayama via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> It works with GIT_EDITOR="emacsclient" or GIT_EDITOR="emacsclient -t"
>
> Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> ---
> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> @@ -26,6 +26,11 @@ open_editor() {
> +if git var GIT_EDITOR | grep ^emacsclient >/dev/null; then
> +       type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
> +       open_emacsclient "$mode" "$@"
> +       exit 0
> +fi

Would it make sense to expand this to support the VISUAL and EDITOR
environment variables too? For instance, since I want to use Emacs for
all my editing needs, I have EDITOR set to reference emacsclient. So,
if GIT_EDITOR is unset, then check VISUAL, and if that is unset, check
EDITOR.

Also, on macOS, I need to set EDITOR (or GIT_EDITOR or VISUAL) to the
full path of emacsclient, but the regex used here is too tight to
recognize that. Perhaps loosening it to just 'emacsclient' would be a
good idea (as it seems unlikely to falsely match any other editor)?

Re: [PATCH 2/2] git-jump: invoke emacsclient

[Yoichi Nakayama]

On Sun, Nov 20, 2022 at 12:53 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> Would it make sense to expand this to support the VISUAL and EDITOR
> environment variables too? For instance, since I want to use Emacs for
> all my editing needs, I have EDITOR set to reference emacsclient. So,
> if GIT_EDITOR is unset, then check VISUAL, and if that is unset, check
> EDITOR.

"git var GIT_EDITOR" does respect VISUAL and EDITOR environment variables.
See git-var(1) reference manual.

> Also, on macOS, I need to set EDITOR (or GIT_EDITOR or VISUAL) to the
> full path of emacsclient, but the regex used here is too tight to
> recognize that. Perhaps loosening it to just 'emacsclient' would be a
> good idea (as it seems unlikely to falsely match any other editor)?

It is a good idea. Thanks!
-- 
Yoichi NAKAYAMA

Re: [PATCH 2/2] git-jump: invoke emacsclient

[Eric Sunshine]

On Sat, Nov 19, 2022 at 6:44 PM Yoichi Nakayama
<yoichi.nakayama@gmail.com> wrote:
> On Sun, Nov 20, 2022 at 12:53 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > Would it make sense to expand this to support the VISUAL and EDITOR
> > environment variables too? For instance, since I want to use Emacs for
> > all my editing needs, I have EDITOR set to reference emacsclient. So,
> > if GIT_EDITOR is unset, then check VISUAL, and if that is unset, check
> > EDITOR.
>
> "git var GIT_EDITOR" does respect VISUAL and EDITOR environment variables.
> See git-var(1) reference manual.

Excellent.

> > Also, on macOS, I need to set EDITOR (or GIT_EDITOR or VISUAL) to the
> > full path of emacsclient, but the regex used here is too tight to
> > recognize that. Perhaps loosening it to just 'emacsclient' would be a
> > good idea (as it seems unlikely to falsely match any other editor)?
>
> It is a good idea. Thanks!

Thanks.

Re: [PATCH 2/2] git-jump: invoke emacsclient

[Yoichi Nakayama]

> > Also, on macOS, I need to set EDITOR (or GIT_EDITOR or VISUAL) to the
> > full path of emacsclient, but the regex used here is too tight to
> > recognize that. Perhaps loosening it to just 'emacsclient' would be a
> > good idea (as it seems unlikely to falsely match any other editor)?
>
> It is a good idea. Thanks!

I found we can support both emacs and emacsclient by using --eval
option instead of -e option.
I'll deal with it.

On Sun, Nov 20, 2022 at 8:59 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sat, Nov 19, 2022 at 6:44 PM Yoichi Nakayama
> <yoichi.nakayama@gmail.com> wrote:
> > On Sun, Nov 20, 2022 at 12:53 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > Would it make sense to expand this to support the VISUAL and EDITOR
> > > environment variables too? For instance, since I want to use Emacs for
> > > all my editing needs, I have EDITOR set to reference emacsclient. So,
> > > if GIT_EDITOR is unset, then check VISUAL, and if that is unset, check
> > > EDITOR.
> >
> > "git var GIT_EDITOR" does respect VISUAL and EDITOR environment variables.
> > See git-var(1) reference manual.
>
> Excellent.
>
> > > Also, on macOS, I need to set EDITOR (or GIT_EDITOR or VISUAL) to the
> > > full path of emacsclient, but the regex used here is too tight to
> > > recognize that. Perhaps loosening it to just 'emacsclient' would be a
> > > good idea (as it seems unlikely to falsely match any other editor)?
> >
> > It is a good idea. Thanks!
>
> Thanks.



-- 
Yoichi NAKAYAMA

[PATCH v2 0/2] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Yoichi Nakayama (2):
  git-jump: add an optional argument 'stdout'
  git-jump: invoke emacsclient

 contrib/git-jump/README   |  9 ++++++++-
 contrib/git-jump/git-jump | 22 +++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v1:

 1:  e56858a3eb2 = 1:  e56858a3eb2 git-jump: add an optional argument 'stdout'
 2:  ed19668db86 ! 2:  72c4fd5532b git-jump: invoke emacsclient
     @@ contrib/git-jump/git-jump: if test "$mode" = "stdout"; then
       	exit 0
       fi
       
     -+if git var GIT_EDITOR | grep ^emacsclient >/dev/null; then
     ++if git var GIT_EDITOR | grep emacsclient >/dev/null; then
      +	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
      +	open_emacsclient "$mode" "$@"
      +	exit 0

-- 
gitgitgadget

[PATCH v2 1/2] git-jump: add an optional argument 'stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   |  9 ++++++++-
 contrib/git-jump/git-jump | 11 ++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..6aaa6a928d2 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,13 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument 'stdout' to print the listing to
+standard output. You can use it with M-x grep on Emacs.
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump stdout <mode>"
+Run grep (like this): git jump stdout diff
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +107,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..a907f69304d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `stdout` is given, print the quickfix
+lines to standard output.
 EOF
 }
 
@@ -69,6 +72,12 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+if test "$mode" = "stdout"; then
+	mode=$1; shift
+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+	"mode_$mode" "$@" 2>/dev/null
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

Re: [PATCH v2 1/2] git-jump: add an optional argument 'stdout'

[Junio C Hamano]

"Yoichi Nakayama via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +You can use the optional argument 'stdout' to print the listing to
> +standard output. You can use it with M-x grep on Emacs.

It is unclear what happens when you do not give 'stdout' from the
above description.  You say 'stdout' is a way to tell the command
"to print the listing to standard output", but what is expected to
follow that sentence is left unsaid (i.e. "you can give 'stdout' to
print to stdout, INSTEAD OF DOING X").

Also, 

> +--------------------------------------------------
> +# In Emacs, M-x grep and invoke "git jump stdout <mode>"
> +Run grep (like this): git jump stdout diff

the command line structure of "git jump" being

    git jump <mode> [<args>]

where <mode> is one of 'diff', 'merge', 'grep', it somehow feels
very strange to insert an optional new word, that is not a dashed
option, before the <mode>.  "git jump --stdout diff" might be less
surprising, but I dunno.

But I think this is a good idea.  In fact, it almost feels that the
interface to produce the list of "$file:$line: <blah>" that this
"stdout" mode gives should have been in the command as the lowest
level basic primitive, upon which the feature to drive a specific
editor using such an output file should have been built, and the
current code is backwards in that sense.  Exposing that lower level
primitive directly is a welcome addition.

Re: [PATCH v2 1/2] git-jump: add an optional argument 'stdout'

[Yoichi Nakayama]

On Mon, Nov 21, 2022 at 2:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Yoichi Nakayama via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +You can use the optional argument 'stdout' to print the listing to
> > +standard output. You can use it with M-x grep on Emacs.
>
> It is unclear what happens when you do not give 'stdout' from the
> above description.  You say 'stdout' is a way to tell the command
> "to print the listing to standard output", but what is expected to
> follow that sentence is left unsaid (i.e. "you can give 'stdout' to
> print to stdout, INSTEAD OF DOING X").

I agree. I'll add "instead of feeding it to the editor".

> > +# In Emacs, M-x grep and invoke "git jump stdout <mode>"
> > +Run grep (like this): git jump stdout diff
>
> the command line structure of "git jump" being
>
>     git jump <mode> [<args>]
>
> where <mode> is one of 'diff', 'merge', 'grep', it somehow feels
> very strange to insert an optional new word, that is not a dashed
> option, before the <mode>.  "git jump --stdout diff" might be less
> surprising, but I dunno.
>
> But I think this is a good idea.  In fact, it almost feels that the
> interface to produce the list of "$file:$line: <blah>" that this
> "stdout" mode gives should have been in the command as the lowest
> level basic primitive, upon which the feature to drive a specific
> editor using such an output file should have been built, and the
> current code is backwards in that sense.  Exposing that lower level
> primitive directly is a welcome addition.

Thank you for your thoughtful consideration.
The value of git-jump is to launch the editor quickly from the command line.
I think it's rare to manually use the additional option (e.g. M-x grep
in Emacs).
Therefore, I think there is not much needs to expose it directly.
I'll change it to less surprising form "--stdout".
-- 
Yoichi NAKAYAMA

Re: [PATCH v2 1/2] git-jump: add an optional argument 'stdout'

[Jeff King]

On Mon, Nov 21, 2022 at 02:43:51PM +0900, Junio C Hamano wrote:

> the command line structure of "git jump" being
> 
>     git jump <mode> [<args>]
> 
> where <mode> is one of 'diff', 'merge', 'grep', it somehow feels
> very strange to insert an optional new word, that is not a dashed
> option, before the <mode>.  "git jump --stdout diff" might be less
> surprising, but I dunno.

FWIW, I had the same reaction: it should be --stdout.

> But I think this is a good idea.  In fact, it almost feels that the
> interface to produce the list of "$file:$line: <blah>" that this
> "stdout" mode gives should have been in the command as the lowest
> level basic primitive, upon which the feature to drive a specific
> editor using such an output file should have been built, and the
> current code is backwards in that sense.  Exposing that lower level
> primitive directly is a welcome addition.

The main reason I didn't start with that primitive is because the
initial outputs were pretty simple and based around "grep -n" output.
But they've gotten a bit more complicated over time, especially the
"diff" one. I agree that just dumping the quickfix list is a good
feature to have.

-Peff

[PATCH v2 2/2] git-jump: invoke emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacsclient" or GIT_EDITOR="emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index a907f69304d..f267eac2233 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -26,6 +26,11 @@ open_editor() {
 	eval "$editor -q \$1"
 }
 
+open_emacsclient() {
+	editor=`git var GIT_EDITOR`
+	eval "$editor -e \"(prog1 (switch-to-buffer-other-frame (grep \\\"git jump stdout $@\\\")) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
+}
+
 mode_diff() {
 	git diff --no-prefix --relative "$@" |
 	perl -ne '
@@ -79,6 +84,12 @@ if test "$mode" = "stdout"; then
 	exit 0
 fi
 
+if git var GIT_EDITOR | grep emacsclient >/dev/null; then
+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+	open_emacsclient "$mode" "$@"
+	exit 0
+fi
+
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
 type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
-- 
gitgitgadget

[PATCH v3 0/2] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Yoichi Nakayama (2):
  git-jump: add an optional argument '--stdout'
  git-jump: invoke emacs/emacsclient

 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 23 ++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v2:

 1:  e56858a3eb2 ! 1:  ccfea26de33 git-jump: add an optional argument 'stdout'
     @@ Metadata
      Author: Yoichi Nakayama <yoichi.nakayama@gmail.com>
      
       ## Commit message ##
     -    git-jump: add an optional argument 'stdout'
     +    git-jump: add an optional argument '--stdout'
      
          It can be used with M-x grep on Emacs.
      
     @@ contrib/git-jump/README: git jump grep -i foo_bar
       git config jump.grepCmd "ag --column"
       --------------------------------------------------
       
     -+You can use the optional argument 'stdout' to print the listing to
     -+standard output. You can use it with M-x grep on Emacs.
     ++You can use the optional argument '--stdout' to print the listing to
     ++standard output instead of feeding it to the editor. You can use the
     ++argument with M-x grep on Emacs:
      +
      +--------------------------------------------------
     -+# In Emacs, M-x grep and invoke "git jump stdout <mode>"
     -+Run grep (like this): git jump stdout diff
     ++# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
     ++Run grep (like this): git jump --stdout diff
      +--------------------------------------------------
       
       Related Programs
     @@ contrib/git-jump/git-jump
       usage() {
       	cat <<\EOF
      -usage: git jump <mode> [<args>]
     -+usage: git jump [stdout] <mode> [<args>]
     ++usage: git jump [--stdout] <mode> [<args>]
       
       Jump to interesting elements in an editor.
       The <mode> parameter is one of:
     @@ contrib/git-jump/git-jump: grep: elements are grep hits. Arguments are given to
       
       ws: elements are whitespace errors. Arguments are given to diff --check.
      +
     -+If the optional argument `stdout` is given, print the quickfix
     -+lines to standard output.
     ++If the optional argument `--stdout` is given, print the quickfix
     ++lines to standard output instead of feeding it to the editor.
       EOF
       }
       
     @@ contrib/git-jump/git-jump: if test $# -lt 1; then
       	exit 1
       fi
       mode=$1; shift
     -+if test "$mode" = "stdout"; then
     ++if test "$mode" = "--stdout"; then
      +	mode=$1; shift
      +	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
      +	"mode_$mode" "$@" 2>/dev/null
 2:  72c4fd5532b ! 2:  b4ad4c083c9 git-jump: invoke emacsclient
     @@ Metadata
      Author: Yoichi Nakayama <yoichi.nakayama@gmail.com>
      
       ## Commit message ##
     -    git-jump: invoke emacsclient
     +    git-jump: invoke emacs/emacsclient
      
     -    It works with GIT_EDITOR="emacsclient" or GIT_EDITOR="emacsclient -t"
     +    It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"
      
          Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
      
     @@ contrib/git-jump/git-jump: open_editor() {
       	eval "$editor -q \$1"
       }
       
     -+open_emacsclient() {
     ++open_emacs() {
      +	editor=`git var GIT_EDITOR`
     -+	eval "$editor -e \"(prog1 (switch-to-buffer-other-frame (grep \\\"git jump stdout $@\\\")) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
     ++	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"git jump --stdout $@\\\" 'grep-mode)) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
      +}
      +
       mode_diff() {
       	git diff --no-prefix --relative "$@" |
       	perl -ne '
     -@@ contrib/git-jump/git-jump: if test "$mode" = "stdout"; then
     +@@ contrib/git-jump/git-jump: if test "$mode" = "--stdout"; then
       	exit 0
       fi
       
     -+if git var GIT_EDITOR | grep emacsclient >/dev/null; then
     ++# For emacs/emacsclient, call "git jump --stdout" from inside of them.
     ++if git var GIT_EDITOR | grep emacs >/dev/null; then
      +	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
     -+	open_emacsclient "$mode" "$@"
     ++	open_emacs "$mode" "$@"
      +	exit 0
      +fi
      +

-- 
gitgitgadget

[PATCH v3 1/2] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 11 ++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..0340980959b 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,14 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument '--stdout' to print the listing to
+standard output instead of feeding it to the editor. You can use the
+argument with M-x grep on Emacs:
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
+Run grep (like this): git jump --stdout diff
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +108,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..091d1add0ec 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [--stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `--stdout` is given, print the quickfix
+lines to standard output instead of feeding it to the editor.
 EOF
 }
 
@@ -69,6 +72,12 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+if test "$mode" = "--stdout"; then
+	mode=$1; shift
+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+	"mode_$mode" "$@" 2>/dev/null
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

Re: [PATCH v3 1/2] git-jump: add an optional argument '--stdout'

[Jeff King]

On Mon, Nov 21, 2022 at 12:26:59PM +0000, Yoichi Nakayama via GitGitGadget wrote:

> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> 
> It can be used with M-x grep on Emacs.

Thanks, I like what this feature is doing overall, but I have some small
nits about the implementation.

> +You can use the optional argument '--stdout' to print the listing to
> +standard output instead of feeding it to the editor. You can use the
> +argument with M-x grep on Emacs:
> +
> +--------------------------------------------------
> +# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
> +Run grep (like this): git jump --stdout diff
> +--------------------------------------------------

This example confused me because it says "run grep", but then runs a
diff jump. But maybe this is because it means to run the emacs grep
command? I don't use emacs, so it may make more sense to somebody who
does.

> @@ -69,6 +72,12 @@ if test $# -lt 1; then
>  	exit 1
>  fi
>  mode=$1; shift
> +if test "$mode" = "--stdout"; then
> +	mode=$1; shift
> +	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
> +	"mode_$mode" "$@" 2>/dev/null
> +	exit 0
> +fi

Because this happens after we check that "$1" isn't empty and call
shift, it may not notice if the mode is missing when we do this second
shift. I.e., with your patch I get:

  $ ./git-jump --stdout
  ./git-jump: 76: shift: can't shift that many

when I should get the usage message. We can fix that by parsing out
--stdout before we try to read the mode. It's a little more code, but I
think it nicely sets us up if we ever want to parse more options.

It's also unfortunate that we have to repeat the ugly "type" check
above, which also happens again later, after we make the temp file. I
see why you did it this way; the stdout code path does not want to make
the tempfile. But the code before your patch was silly to do it this
way; we should always have been checking the parameters before making a
tempfile.

I was also puzzled why the stdout mode redirects stderr from the mode
function. Wouldn't the user want to see any errors?

So together, it might look something like this (instead of, rather than
on top of your patch):

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index b9cc602ebf..05a0ff1430 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -67,15 +67,38 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+use_stdout=
+while test $# -gt 0; do
+	case "$1" in
+	--stdout)
+		use_stdout=t
+		shift
+		;;
+	--*)
+		usage >&2
+		exit 1
+		;;
+	*)
+		break
+		;;
+	esac
+done
+
 if test $# -lt 1; then
 	usage >&2
 	exit 1
 fi
+
 mode=$1; shift
+type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+
+if test "$use_stdout" = "t"; then
+	"mode_$mode" "$@"
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
 open_editor "$tmp"

Though I'd perhaps break some of the shuffling into a preparatory patch.
I'm happy to do that separately if you prefer.

-Peff

Re: [PATCH v3 1/2] git-jump: add an optional argument '--stdout'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Nov 21, 2022 at 12:26:59PM +0000, Yoichi Nakayama via GitGitGadget wrote:
>
>> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
>> 
>> It can be used with M-x grep on Emacs.
>
> Thanks, I like what this feature is doing overall, but I have some small
> nits about the implementation.
>
>> +You can use the optional argument '--stdout' to print the listing to
>> +standard output instead of feeding it to the editor. You can use the
>> +argument with M-x grep on Emacs:
>> +
>> +--------------------------------------------------
>> +# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
>> +Run grep (like this): git jump --stdout diff
>> +--------------------------------------------------
>
> This example confused me because it says "run grep", but then runs a
> diff jump. But maybe this is because it means to run the emacs grep
> command? I don't use emacs, so it may make more sense to somebody who
> does.

Yes.  "M-x" gives Emacs users a command line prompt to type (and tab
complete) an Emacs command, and in the above explanation, the user
is running the "grep" command of Emacs, which in turn prompts for a
shell command that produces series of <filename>:<lineno>:<cruft> to
jump around [*].

"M-x grep<RET>git jump --stdout diff<RET>" is what I would have
written on the second line instead of "Run grep (like this)...".

[Footnote]

* People often run "grep -n -r -e <pattern>" but you can run things
like "git grep -n -e <pattern> -- <pathspec>" and "find -name \*.h |
xargs grep -n -e <pattern>".

Re: [PATCH v3 1/2] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama]

On Tue, Nov 22, 2022 at 8:38 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > On Mon, Nov 21, 2022 at 12:26:59PM +0000, Yoichi Nakayama via GitGitGadget wrote:
> >
> >> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> >> +--------------------------------------------------
> >> +# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
> >> +Run grep (like this): git jump --stdout diff
> >> +--------------------------------------------------
> >
> > This example confused me because it says "run grep", but then runs a
> > diff jump. But maybe this is because it means to run the emacs grep
> > command? I don't use emacs, so it may make more sense to somebody who
> > does.
>
> Yes.  "M-x" gives Emacs users a command line prompt to type (and tab
> complete) an Emacs command, and in the above explanation, the user
> is running the "grep" command of Emacs, which in turn prompts for a
> shell command that produces series of <filename>:<lineno>:<cruft> to
> jump around [*].
>
> "M-x grep<RET>git jump --stdout diff<RET>" is what I would have
> written on the second line instead of "Run grep (like this)...".

I think Junio's suggestion of  "M-x grep<RET>git jump --stdout diff<RET>"
is concise and understandable to most Emacs users, so I'd like to adopt it.

Below are the details of what I thought:

By M-x grep<RET>, Emacs displays
      Run grep (like this): grep --color=auto -nH --null -e
where
- "Run grep (like this): " is a prompt (like "$ " in bash).
- "grep --color=auto -nH --null -e " is a part of search command (missing
  keyword part). We can supply "keyword<RET>" to execute the search.
  We can also remove the whole command and replace it with the command
  like "git jump --stdout diff".

So "M-x grep<RET>git jump --stdout diff<RET>" does not represent the
complete procedure. It lacks the operation to remove the default command
(controlled by the grep-command setting). For example, adding C-a C-k
after "M-x grep<RET>" is more accurate, but it feels a bit redundant.
-- 
Yoichi NAKAYAMA

Re: [PATCH v3 1/2] git-jump: add an optional argument '--stdout'

[Jeff King]

On Tue, Nov 22, 2022 at 10:00:45PM +0900, Yoichi Nakayama wrote:

> > "M-x grep<RET>git jump --stdout diff<RET>" is what I would have
> > written on the second line instead of "Run grep (like this)...".
> 
> I think Junio's suggestion of  "M-x grep<RET>git jump --stdout diff<RET>"
> is concise and understandable to most Emacs users, so I'd like to adopt it.
> 
> Below are the details of what I thought:
> 
> By M-x grep<RET>, Emacs displays
>       Run grep (like this): grep --color=auto -nH --null -e
> where
> - "Run grep (like this): " is a prompt (like "$ " in bash).
> - "grep --color=auto -nH --null -e " is a part of search command (missing
>   keyword part). We can supply "keyword<RET>" to execute the search.
>   We can also remove the whole command and replace it with the command
>   like "git jump --stdout diff".

Ah, OK. I am happy with anything that emacs users will understand, and I
trust the two of you to come up with something there. :)

-Peff

Re: [PATCH v3 1/2] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama]

On Tue, Nov 22, 2022 at 3:38 AM Jeff King <peff@peff.net> wrote:
> It's also unfortunate that we have to repeat the ugly "type" check
> above, which also happens again later, after we make the temp file. I
> see why you did it this way; the stdout code path does not want to make
> the tempfile. But the code before your patch was silly to do it this
> way; we should always have been checking the parameters before making a
> tempfile.
>
> I was also puzzled why the stdout mode redirects stderr from the mode
> function. Wouldn't the user want to see any errors?
>
> So together, it might look something like this (instead of, rather than
> on top of your patch):

Thanks. I've applied it.

I was throwing away stderr because Emacs' M-x grep inserted both stdout
and stderr into the output destination, and a perl warning was issued.
However, the warning itself is meaningful, and I thought it would be very bad
not to know the reason when an error occurred, so I came to the conclusion
that stderr should be left as is.
-- 
Yoichi NAKAYAMA

[PATCH v3 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 091d1add0ec..43ca8bb1ee7 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -26,6 +26,11 @@ open_editor() {
 	eval "$editor -q \$1"
 }
 
+open_emacs() {
+	editor=`git var GIT_EDITOR`
+	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"git jump --stdout $@\\\" 'grep-mode)) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
+}
+
 mode_diff() {
 	git diff --no-prefix --relative "$@" |
 	perl -ne '
@@ -79,6 +84,13 @@ if test "$mode" = "--stdout"; then
 	exit 0
 fi
 
+# For emacs/emacsclient, call "git jump --stdout" from inside of them.
+if git var GIT_EDITOR | grep emacs >/dev/null; then
+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+	open_emacs "$mode" "$@"
+	exit 0
+fi
+
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
 type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
-- 
gitgitgadget

Re: [PATCH v3 2/2] git-jump: invoke emacs/emacsclient

[Jeff King]

On Mon, Nov 21, 2022 at 12:27:00PM +0000, Yoichi Nakayama via GitGitGadget wrote:

> +open_emacs() {
> +	editor=`git var GIT_EDITOR`
> +	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"git jump --stdout $@\\\" 'grep-mode)) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
> +}

I think this subjects the user's arguments to an extra round of
whitespace splitting. E.g., if I do:

  git jump grep 'foo bar'

then the emacs command will see two arguments. The first is "--eval",
and the second is the whole string:

  prog1 (switch-to-buffer-other-frame (compilation-start "git jump --stdout grep foo bar" 'grep-mode)) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))

But now we've lost fact that "foo bar" was a single string, and git-grep
will complain (because it treats "bar" as a file to look in, which does
not exist).

You'll have to either shell-quote the contents, or stuff it in a
tempfile and read it with something like "cat /path/to/tempfile" in the
emacs command (though I'm not sure if that would be racy when you're
using something like emacsclient which may exit before the main emacs
process runs the command).

-Peff

Re: [PATCH v3 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Tue, Nov 22, 2022 at 3:50 AM Jeff King <peff@peff.net> wrote:
> You'll have to either shell-quote the contents, or stuff it in a
> tempfile and read it with something like "cat /path/to/tempfile" in the
> emacs command (though I'm not sure if that would be racy when you're
> using something like emacsclient which may exit before the main emacs
> process runs the command).

As you pointed out, the previous implementation has a problem in cases such as:
      git jump grep "hello world"

The reason I avoided using temporary files in the previous implementation was
a race condition when editor="emacsclient" (emacsclient exits without
waiting for
the asynchronous execution of the external command). But I found a way around
the race condition, so I'll change to use cat on a temporary file.

Thanks,
-- 
Yoichi NAKAYAMA

[PATCH v4 0/2] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Yoichi Nakayama (2):
  git-jump: add an optional argument '--stdout'
  git-jump: invoke emacs/emacsclient

 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 40 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 2 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v3:

 1:  ccfea26de33 ! 1:  446777d300d git-jump: add an optional argument '--stdout'
     @@ contrib/git-jump/README: git jump grep -i foo_bar
      +
      +--------------------------------------------------
      +# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
     -+Run grep (like this): git jump --stdout diff
     ++M-x grep<RET>git jump --stdout diff<RET>
      +--------------------------------------------------
       
       Related Programs
     @@ contrib/git-jump/git-jump: grep: elements are grep hits. Arguments are given to
       EOF
       }
       
     -@@ contrib/git-jump/git-jump: if test $# -lt 1; then
     +@@ contrib/git-jump/git-jump: mode_ws() {
     + 	git diff --check "$@"
     + }
     + 
     ++use_stdout=
     ++while test $# -gt 0; do
     ++	case "$1" in
     ++	--stdout)
     ++		use_stdout=t
     ++		shift
     ++		;;
     ++	--*)
     ++		usage >&2
     ++		exit 1
     ++		;;
     ++	*)
     ++		break
     ++		;;
     ++	esac
     ++done
     + if test $# -lt 1; then
     + 	usage >&2
       	exit 1
       fi
       mode=$1; shift
     -+if test "$mode" = "--stdout"; then
     -+	mode=$1; shift
     -+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
     -+	"mode_$mode" "$@" 2>/dev/null
     ++if test "$use_stdout" = "t"; then
     ++	"mode_$mode" "$@"
      +	exit 0
      +fi
       
 2:  b4ad4c083c9 ! 2:  2f0bffb484b git-jump: invoke emacs/emacsclient
     @@ contrib/git-jump/git-jump: open_editor() {
       }
       
      +open_emacs() {
     ++	# Supported editor values are:
     ++	# - emacs
     ++	# - emacsclient
     ++	# - emacsclient -t
      +	editor=`git var GIT_EDITOR`
     -+	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"git jump --stdout $@\\\" 'grep-mode)) (delete-other-windows) (select-frame-set-input-focus (selected-frame)))\""
     ++	# Wait for completion of the asynchronously executed process
     ++	# to avoid race conditions in case of "emacsclient".
     ++	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"cat $@\\\" 'grep-mode)) (delete-other-windows) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
      +}
      +
       mode_diff() {
       	git diff --no-prefix --relative "$@" |
       	perl -ne '
     -@@ contrib/git-jump/git-jump: if test "$mode" = "--stdout"; then
     - 	exit 0
     - fi
     - 
     -+# For emacs/emacsclient, call "git jump --stdout" from inside of them.
     +@@ contrib/git-jump/git-jump: tmp=`mktemp -t git-jump.XXXXXX` || exit 1
     + type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
     + "mode_$mode" "$@" >"$tmp"
     + test -s "$tmp" || exit 0
      +if git var GIT_EDITOR | grep emacs >/dev/null; then
     -+	type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
     -+	open_emacs "$mode" "$@"
     ++	open_emacs "$tmp"
      +	exit 0
      +fi
     -+
     - trap 'rm -f "$tmp"' 0 1 2 3 15
     - tmp=`mktemp -t git-jump.XXXXXX` || exit 1
     - type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
     + open_editor "$tmp"

-- 
gitgitgadget

[PATCH v4 1/2] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..3211841305f 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,14 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument '--stdout' to print the listing to
+standard output instead of feeding it to the editor. You can use the
+argument with M-x grep on Emacs:
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
+M-x grep<RET>git jump --stdout diff<RET>
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +108,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..babb3b5c68d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [--stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `--stdout` is given, print the quickfix
+lines to standard output instead of feeding it to the editor.
 EOF
 }
 
@@ -64,11 +67,31 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+use_stdout=
+while test $# -gt 0; do
+	case "$1" in
+	--stdout)
+		use_stdout=t
+		shift
+		;;
+	--*)
+		usage >&2
+		exit 1
+		;;
+	*)
+		break
+		;;
+	esac
+done
 if test $# -lt 1; then
 	usage >&2
 	exit 1
 fi
 mode=$1; shift
+if test "$use_stdout" = "t"; then
+	"mode_$mode" "$@"
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

Re: [PATCH v4 1/2] git-jump: add an optional argument '--stdout'

[Jeff King]

On Tue, Nov 22, 2022 at 02:18:22PM +0000, Yoichi Nakayama via GitGitGadget wrote:

> @@ -64,11 +67,31 @@ mode_ws() {
>  	git diff --check "$@"
>  }
>  
> +use_stdout=
> +while test $# -gt 0; do
> +	case "$1" in
> +	--stdout)
> +		use_stdout=t
> +		shift
> +		;;
> +	--*)
> +		usage >&2
> +		exit 1
> +		;;
> +	*)
> +		break
> +		;;
> +	esac
> +done
>  if test $# -lt 1; then
>  	usage >&2
>  	exit 1
>  fi
>  mode=$1; shift
> +if test "$use_stdout" = "t"; then
> +	"mode_$mode" "$@"
> +	exit 0
> +fi

Thanks, this looks pretty good. I think we'd want this on top.

-- >8 --
Subject: git-jump: move valid-mode check earlier

We check if the "mode" argument supplied by the user is valid by seeing
if we have a mode_$mode function defined. But we don't do that until
after creating the tempfile. This is wasteful (we create a tempfile but
never use it), and makes it harder to add new options (the recent stdout
option exits before creating the tempfile, so it misses the check and
"git jump --stdout foo" will produce "git-jump: 92: mode_foo: not found"
rather than the regular usage message).

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/git-jump/git-jump | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index babb3b5c68..cc97b0dcf0 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -88,14 +88,15 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+
 if test "$use_stdout" = "t"; then
 	"mode_$mode" "$@"
 	exit 0
 fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
 open_editor "$tmp"
-- 
2.38.1.970.g3b99f132c8

[PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index babb3b5c68d..bfd759aa4b2 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -26,6 +26,17 @@ open_editor() {
 	eval "$editor -q \$1"
 }
 
+open_emacs() {
+	# Supported editor values are:
+	# - emacs
+	# - emacsclient
+	# - emacsclient -t
+	editor=`git var GIT_EDITOR`
+	# Wait for completion of the asynchronously executed process
+	# to avoid race conditions in case of "emacsclient".
+	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"cat $@\\\" 'grep-mode)) (delete-other-windows) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
+}
+
 mode_diff() {
 	git diff --no-prefix --relative "$@" |
 	perl -ne '
@@ -98,4 +109,8 @@ tmp=`mktemp -t git-jump.XXXXXX` || exit 1
 type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
+if git var GIT_EDITOR | grep emacs >/dev/null; then
+	open_emacs "$tmp"
+	exit 0
+fi
 open_editor "$tmp"
-- 
gitgitgadget

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Phillip Wood]

Hi Yoichi

On 22/11/2022 14:18, Yoichi Nakayama via GitGitGadget wrote:
> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> 
> It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Thanks for working on this, I'm looking forward to being able to use 
"git jump" with emacs.

> Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> ---
>   contrib/git-jump/git-jump | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index babb3b5c68d..bfd759aa4b2 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -26,6 +26,17 @@ open_editor() {
>   	eval "$editor -q \$1"
>   }
>   
> +open_emacs() {
> +	# Supported editor values are:
> +	# - emacs
> +	# - emacsclient
> +	# - emacsclient -t
> +	editor=`git var GIT_EDITOR`
> +	# Wait for completion of the asynchronously executed process
> +	# to avoid race conditions in case of "emacsclient".
> +	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"cat $@\\\" 'grep-mode)) (delete-other-windows) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""

I just tried this out in a frame (window for non emacs users) showing 
two files and the (delete-other-windows) call replaced both of them with 
the grep buffer. It would be nicer if it created a new window in the 
current frame or showed the grep buffer in one of the existing windows. 
If I delete (delete-other-windows) then the first time I run "git jump" 
it shows the grep buffer in the frame I already have open, but then if I 
run it again without closing the grep buffer it opens a new frame. I 
wonder if it would be better just to close the buffer if it exists 
before creating the new one or pass NAME-FUNCTION argument to 
compilation-start that creates unique names.

I'm using emacsclient as my editor and when I run "git jump" it prints

#<buffer *grep*>

in the terminal (presumably because that is the return value of 
select-frame-set-input-focus)

Could we read the file and set the buffer's mode to grep-mode (or 
compilation-mode?) without forking cat?

Best Wishes

Phillip

> +}
> +
>   mode_diff() {
>   	git diff --no-prefix --relative "$@" |
>   	perl -ne '
> @@ -98,4 +109,8 @@ tmp=`mktemp -t git-jump.XXXXXX` || exit 1
>   type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
>   "mode_$mode" "$@" >"$tmp"
>   test -s "$tmp" || exit 0
> +if git var GIT_EDITOR | grep emacs >/dev/null; then
> +	open_emacs "$tmp"
> +	exit 0
> +fi
>   open_editor "$tmp"

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Wed, Nov 23, 2022 at 1:40 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> > +     # Wait for completion of the asynchronously executed process
> > +     # to avoid race conditions in case of "emacsclient".
> > +     eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"cat $@\\\" 'grep-mode)) (delete-other-windows) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
>
> I just tried this out in a frame (window for non emacs users) showing
> two files and the (delete-other-windows) call replaced both of them with
> the grep buffer. It would be nicer if it created a new window in the
> current frame or showed the grep buffer in one of the existing windows.

Thanks for your feedback.

The first point is that you want to keep the same window configuration
as before you do git jump, and reuse existing window (like M-x grep), right?

I think "(delete-other-windows)" was superfluous, so I'll remove it.
Will it do what you want?
- In case of  editor="emacsclient", it will try to keep window configuration.
- In case of editor="emacsclient -t" and editor="emacs", it will
create two window
configuration (up and down).

> If I delete (delete-other-windows) then the first time I run "git jump"
> it shows the grep buffer in the frame I already have open, but then if I
> run it again without closing the grep buffer it opens a new frame. I
> wonder if it would be better just to close the buffer if it exists
> before creating the new one or pass NAME-FUNCTION argument to
> compilation-start that creates unique names.

I've also seen a new frame being created unintentionally.
It is caused by the wrong use of switch-to-buffer-other-frame.
I'll try to fix.
-- 
Yoichi NAKAYAMA

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Jeff King]

On Tue, Nov 22, 2022 at 02:18:23PM +0000, Yoichi Nakayama via GitGitGadget wrote:

> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index babb3b5c68d..bfd759aa4b2 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -26,6 +26,17 @@ open_editor() {
>  	eval "$editor -q \$1"
>  }
>  
> +open_emacs() {
> +	# Supported editor values are:
> +	# - emacs
> +	# - emacsclient
> +	# - emacsclient -t
> +	editor=`git var GIT_EDITOR`
> +	# Wait for completion of the asynchronously executed process
> +	# to avoid race conditions in case of "emacsclient".
> +	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"cat $@\\\" 'grep-mode)) (delete-other-windows) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
> +}

Hmm, I know I suggested using a temporary file since "cat $tmpfile"
should be pretty safe. But it does still have problems if your tmp
directory has spaces. Or even other metacharacters, which I think will
be interpreted by the eval, since $@ is expanded in the outermost level
of the shell.

Those are fairly unlikely, but we could handle it. I think you'd need
something like:

	open_emacs() {
		quoted_args=
		for i in "$@"; do
			quoted_args="$quoted_args '$(printf %s "$i" | sed "s/'/'\\\\''/g")'"
		done
		eval "$editor --eval \"...\\\"cat \$quoted_args\\\"...\""
	}

which you can test with:

	cat >fake-emacs <<-\EOF
	#!/bin/sh
	echo "fake-emacs got args: "
	for i in "$@"; do
		echo "arg: $i"
	done
	EOF
	chmod +x fake-emacs

	editor=./fake-emacs
	open_emacs 'multiple args' 'with spaces'
	open_emacs '$dollar is ok because we use single-quotes'
	open_emacs "but 'single quotes' themselves need quoted"

Though it's possible you also need to be adding an extra layer of
quoting due to emacs parsing the string. So you'd probably need to
additionally escape double-quotes and backslashes, perhaps by changing
the sed invocation to:

  sed -e 's/\\/\\\\/g' \
      -e "s/'/'\\\\''/g" \
      -e 's/"/\\"/g'

Which is kind of horrific, but I think is bullet-proof.

Like I said, it's not that likely that somebody's tempfile path would
need all that (though spaces aren't totally out of the question,
especially on Windows). But...

If we have bullet-proof quoting, then you could go back to skipping the
tempfile for emacs, which avoids the race and sleep that you have here.

> @@ -98,4 +109,8 @@ tmp=`mktemp -t git-jump.XXXXXX` || exit 1
>  type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
>  "mode_$mode" "$@" >"$tmp"
>  test -s "$tmp" || exit 0
> +if git var GIT_EDITOR | grep emacs >/dev/null; then
> +	open_emacs "$tmp"
> +	exit 0
> +fi
>  open_editor "$tmp"

If we are going to use a tempfile, this logic should probably get
stuffed into open_editor itself, like:

  open_editor() {
          editor=`git var GIT_EDITOR`
          case "$editor" in
          *emacs*)
                  ...do-the-emacs-thing...
          *)
                  # assume anything else is vi-compatible
                  eval "$editor -q \$1"
          esac
  }

but if you take the quoting suggestion above, then open_emacs() would
continue to be a top-level thing, before we even create the tempfile.

-Peff

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Wed, Nov 23, 2022 at 3:54 AM Jeff King <peff@peff.net> wrote:
> Hmm, I know I suggested using a temporary file since "cat $tmpfile"
> should be pretty safe. But it does still have problems if your tmp
> directory has spaces. Or even other metacharacters, which I think will
> be interpreted by the eval, since $@ is expanded in the outermost level
> of the shell.

Right. But the problem is not specific to emacs (it happens in vim too).
Let's fix it another time (as you noted, that's pretty unlikely, and we may
not even need to fix it).

> If we are going to use a tempfile, this logic should probably get
> stuffed into open_editor itself, like:
>
>   open_editor() {
>           editor=`git var GIT_EDITOR`
>           case "$editor" in
>           *emacs*)
>                   ...do-the-emacs-thing...
>           *)
>                   # assume anything else is vi-compatible
>                   eval "$editor -q \$1"
>           esac
>   }

Sure.
-- 
Yoichi NAKAYAMA

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Jeff King]

On Wed, Nov 23, 2022 at 02:33:50PM +0900, Yoichi Nakayama wrote:

> On Wed, Nov 23, 2022 at 3:54 AM Jeff King <peff@peff.net> wrote:
> > Hmm, I know I suggested using a temporary file since "cat $tmpfile"
> > should be pretty safe. But it does still have problems if your tmp
> > directory has spaces. Or even other metacharacters, which I think will
> > be interpreted by the eval, since $@ is expanded in the outermost level
> > of the shell.
> 
> Right. But the problem is not specific to emacs (it happens in vim too).
> Let's fix it another time (as you noted, that's pretty unlikely, and we may
> not even need to fix it).

Good point. The vim version is easier to fix (we just need to
double-quote \$1 inside the eval), but the fact that nobody has
complained is an indication that it does not really matter.

-Peff

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Thu, Nov 24, 2022 at 10:09 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Nov 23, 2022 at 02:33:50PM +0900, Yoichi Nakayama wrote:
>
> > On Wed, Nov 23, 2022 at 3:54 AM Jeff King <peff@peff.net> wrote:
> > > Hmm, I know I suggested using a temporary file since "cat $tmpfile"
> > > should be pretty safe. But it does still have problems if your tmp
> > > directory has spaces. Or even other metacharacters, which I think will
> > > be interpreted by the eval, since $@ is expanded in the outermost level
> > > of the shell.
> >
> > Right. But the problem is not specific to emacs (it happens in vim too).
> > Let's fix it another time (as you noted, that's pretty unlikely, and we may
> > not even need to fix it).
>
> Good point. The vim version is easier to fix (we just need to
> double-quote \$1 inside the eval), but the fact that nobody has
> complained is an indication that it does not really matter.

I've confirmed the vim version is fixed by
    eval "$editor -q \"\$1\""

With your hint, I found the emacs version can be fixed
by single-quoting the variable (I found a mistake in the
emacs version. Since there is only one argument, I
should use $1 instead of $@. I'll fix it.), and the vim
version can be also in the similar form with single quote:
    eval "$editor -q '$1'"

The original vim version used the notation \$1 instead of $1.
I'm worried that the emacs version might need the backslash.
What does the backslash mean? Is it necessary?
-- 
Yoichi NAKAYAMA

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Thu, Nov 24, 2022 at 9:32 PM Yoichi Nakayama
<yoichi.nakayama@gmail.com> wrote:
>
> On Thu, Nov 24, 2022 at 10:09 AM Jeff King <peff@peff.net> wrote:
> >
> > On Wed, Nov 23, 2022 at 02:33:50PM +0900, Yoichi Nakayama wrote:
> >
> > > On Wed, Nov 23, 2022 at 3:54 AM Jeff King <peff@peff.net> wrote:
> > > > Hmm, I know I suggested using a temporary file since "cat $tmpfile"
> > > > should be pretty safe. But it does still have problems if your tmp
> > > > directory has spaces. Or even other metacharacters, which I think will
> > > > be interpreted by the eval, since $@ is expanded in the outermost level
> > > > of the shell.
> > >
> > > Right. But the problem is not specific to emacs (it happens in vim too).
> > > Let's fix it another time (as you noted, that's pretty unlikely, and we may
> > > not even need to fix it).
> >
> > Good point. The vim version is easier to fix (we just need to
> > double-quote \$1 inside the eval), but the fact that nobody has
> > complained is an indication that it does not really matter.
>
> I've confirmed the vim version is fixed by
>     eval "$editor -q \"\$1\""
>
> With your hint, I found the emacs version can be fixed
> by single-quoting the variable (I found a mistake in the
> emacs version. Since there is only one argument, I
> should use $1 instead of $@. I'll fix it.), and the vim
> version can be also in the similar form with single quote:
>     eval "$editor -q '$1'"
>
> The original vim version used the notation \$1 instead of $1.
> I'm worried that the emacs version might need the backslash.
> What does the backslash mean? Is it necessary?

I found the answer myself. The backslash is to leave the
evaluation of the argument to the 'eval' execution.
And another question arose. Why do we use eval?
What is the difference from running it directly like below?
    $editor -q $1

-- 
Yoichi NAKAYAMA

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Jeff King]

On Thu, Nov 24, 2022 at 09:58:04PM +0900, Yoichi Nakayama wrote:

> > The original vim version used the notation \$1 instead of $1.
> > I'm worried that the emacs version might need the backslash.
> > What does the backslash mean? Is it necessary?
> 
> I found the answer myself. The backslash is to leave the
> evaluation of the argument to the 'eval' execution.
> And another question arose. Why do we use eval?
> What is the difference from running it directly like below?
>     $editor -q $1

The value of $editor is not a single program name, but is itself a shell
command. So you could imagine:

  git config core.editor "some_command --with --args"

or even more complicated shell hackery. From within Git, we'd run it as:

  sh -c "some_command --with --args"

but when you are in a shell already, "eval" is a more efficient way of
doing the same.

-Peff

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Jeff King]

On Thu, Nov 24, 2022 at 09:32:45PM +0900, Yoichi Nakayama wrote:

> > Good point. The vim version is easier to fix (we just need to
> > double-quote \$1 inside the eval), but the fact that nobody has
> > complained is an indication that it does not really matter.
> 
> I've confirmed the vim version is fixed by
>     eval "$editor -q \"\$1\""
> 
> With your hint, I found the emacs version can be fixed
> by single-quoting the variable (I found a mistake in the
> emacs version. Since there is only one argument, I
> should use $1 instead of $@. I'll fix it.), and the vim
> version can be also in the similar form with single quote:
>     eval "$editor -q '$1'"

This isn't quite a full solution, though. The "$1" is expanded by the
outer double-quoted string, which is then fed to eval. The inner
single-quotes make most characters literally except for single-quotes
themselves. So if $1 has single-quotes, the eval will barf due to bad
syntax.

> The original vim version used the notation \$1 instead of $1.
> I'm worried that the emacs version might need the backslash.
> What does the backslash mean? Is it necessary?

As you figured out in the other email, it inhibits the outer layer of
expansion, and lets the eval expand it. This is the easiest way to pass
things through levels of shell evals (since otherwise you have to
actually quote, which is a real pain).

None of this is sufficient for your emacs example, though. There you
have three levels of quoting:

  - getting the argument intact into the eval; this can use the "\$"
    trick

  - the argument then appears inside a double-quoted string which will
    be evaluated by emacs. You'd want to protect it against
    double-quotes and presumably backslashes.

  - emacs will then execute the final command, presumably you a shell.
    So you'd want to protect against expansion in that shell. The
    easiest way to do that is usually to wrap each argument in
    single-quotes, and quoting against interior single quotes (by ending
    single-quote, adding a single backslashed single-quote, and then
    re-opening the single-quote). It's horribly ugly, but is (AFAIK) the
    shortest way to quote shell arguments, and what we usually do in
    Git.

Those are the three tricks I sent in the earlier email (though looking
at it again, I think the single-quote bits need to come first, so their
backslashes are then quoted to protect against emacs evaluation).

It's all quite confusing, which is why I am OK with just skipping it for
now. ;) The nice thing, though, is that doing the quoting right means
it's safe to get rid of the "cat", which solves your race problems in a
more direct and robust way.

-Peff

Re: [PATCH v4 2/2] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Fri, Nov 25, 2022 at 1:29 AM Jeff King <peff@peff.net> wrote:
> Those are the three tricks I sent in the earlier email (though looking
> at it again, I think the single-quote bits need to come first, so their
> backslashes are then quoted to protect against emacs evaluation).
>
> It's all quite confusing, which is why I am OK with just skipping it for
> now. ;) The nice thing, though, is that doing the quoting right means
> it's safe to get rid of the "cat", which solves your race problems in a
> more direct and robust way.

Thank you for taking the time to explain the details. I don't fully
understand it yet, but I know it's hard to deal with properly.

In PATCH v7, I made it the same level as the vim version (although it
is not strictly at the same level because there is a Emacs Lisp processing).
I skip futher treatment of temporary file paths.

Regards,
-- 
Yoichi NAKAYAMA

[PATCH v5 0/3] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Jeff King (1):
  git-jump: move valid-mode check earlier

Yoichi Nakayama (2):
  git-jump: add an optional argument '--stdout'
  git-jump: invoke emacs/emacsclient

 contrib/git-jump/README   | 10 ++++++++-
 contrib/git-jump/git-jump | 45 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v4:

 1:  446777d300d = 1:  446777d300d git-jump: add an optional argument '--stdout'
 2:  2f0bffb484b ! 2:  972d51888ba git-jump: invoke emacs/emacsclient
     @@
       ## Metadata ##
     -Author: Yoichi Nakayama <yoichi.nakayama@gmail.com>
     +Author: Jeff King <peff@peff.net>
      
       ## Commit message ##
     -    git-jump: invoke emacs/emacsclient
     +    git-jump: move valid-mode check earlier
      
     -    It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"
     +    We check if the "mode" argument supplied by the user is valid by seeing
     +    if we have a mode_$mode function defined. But we don't do that until
     +    after creating the tempfile. This is wasteful (we create a tempfile but
     +    never use it), and makes it harder to add new options (the recent stdout
     +    option exits before creating the tempfile, so it misses the check and
     +    "git jump --stdout foo" will produce "git-jump: 92: mode_foo: not found"
     +    rather than the regular usage message).
      
     -    Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
     +    Signed-off-by: Jeff King <peff@peff.net>
      
       ## contrib/git-jump/git-jump ##
     -@@ contrib/git-jump/git-jump: open_editor() {
     - 	eval "$editor -q \$1"
     - }
     - 
     -+open_emacs() {
     -+	# Supported editor values are:
     -+	# - emacs
     -+	# - emacsclient
     -+	# - emacsclient -t
     -+	editor=`git var GIT_EDITOR`
     -+	# Wait for completion of the asynchronously executed process
     -+	# to avoid race conditions in case of "emacsclient".
     -+	eval "$editor --eval \"(prog1 (switch-to-buffer-other-frame (compilation-start \\\"cat $@\\\" 'grep-mode)) (delete-other-windows) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
     -+}
     +@@ contrib/git-jump/git-jump: if test $# -lt 1; then
     + 	exit 1
     + fi
     + mode=$1; shift
     ++type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
      +
     - mode_diff() {
     - 	git diff --no-prefix --relative "$@" |
     - 	perl -ne '
     -@@ contrib/git-jump/git-jump: tmp=`mktemp -t git-jump.XXXXXX` || exit 1
     - type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
     + if test "$use_stdout" = "t"; then
     + 	"mode_$mode" "$@"
     + 	exit 0
     +@@ contrib/git-jump/git-jump: fi
     + 
     + trap 'rm -f "$tmp"' 0 1 2 3 15
     + tmp=`mktemp -t git-jump.XXXXXX` || exit 1
     +-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
       "mode_$mode" "$@" >"$tmp"
       test -s "$tmp" || exit 0
     -+if git var GIT_EDITOR | grep emacs >/dev/null; then
     -+	open_emacs "$tmp"
     -+	exit 0
     -+fi
       open_editor "$tmp"
 -:  ----------- > 3:  ad7c299cb0f git-jump: invoke emacs/emacsclient

-- 
gitgitgadget

[PATCH v5 1/3] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..3211841305f 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,14 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument '--stdout' to print the listing to
+standard output instead of feeding it to the editor. You can use the
+argument with M-x grep on Emacs:
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
+M-x grep<RET>git jump --stdout diff<RET>
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +108,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..babb3b5c68d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [--stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `--stdout` is given, print the quickfix
+lines to standard output instead of feeding it to the editor.
 EOF
 }
 
@@ -64,11 +67,31 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+use_stdout=
+while test $# -gt 0; do
+	case "$1" in
+	--stdout)
+		use_stdout=t
+		shift
+		;;
+	--*)
+		usage >&2
+		exit 1
+		;;
+	*)
+		break
+		;;
+	esac
+done
 if test $# -lt 1; then
 	usage >&2
 	exit 1
 fi
 mode=$1; shift
+if test "$use_stdout" = "t"; then
+	"mode_$mode" "$@"
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

[PATCH v5 2/3] git-jump: move valid-mode check earlier

[Jeff King via GitGitGadget]

From: Jeff King <peff@peff.net>

We check if the "mode" argument supplied by the user is valid by seeing
if we have a mode_$mode function defined. But we don't do that until
after creating the tempfile. This is wasteful (we create a tempfile but
never use it), and makes it harder to add new options (the recent stdout
option exits before creating the tempfile, so it misses the check and
"git jump --stdout foo" will produce "git-jump: 92: mode_foo: not found"
rather than the regular usage message).

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/git-jump/git-jump | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index babb3b5c68d..cc97b0dcf02 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -88,6 +88,8 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+
 if test "$use_stdout" = "t"; then
 	"mode_$mode" "$@"
 	exit 0
@@ -95,7 +97,6 @@ fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
 open_editor "$tmp"
-- 
gitgitgadget

[PATCH v5 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index cc97b0dcf02..316e9628725 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -23,7 +23,22 @@ EOF
 
 open_editor() {
 	editor=`git var GIT_EDITOR`
-	eval "$editor -q \$1"
+	case "$editor" in
+	*emacs*)
+		# Supported editor values are:
+		# - emacs
+		# - emacsclient
+		# - emacsclient -t
+		#
+		# Wait for completion of the asynchronously executed process
+		# to avoid race conditions in case of "emacsclient".
+		eval "$editor --eval \"(prog1 (pop-to-buffer (compilation-start \\\"cat $@\\\" 'grep-mode)) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
+		;;
+	*)
+		# assume anything else is vi-compatible
+		eval "$editor -q \$1"
+		;;
+	esac
 }
 
 mode_diff() {
-- 
gitgitgadget

Re: [PATCH v5 3/3] git-jump: invoke emacs/emacsclient

[Phillip Wood]

Hi Yoichi

On 23/11/2022 07:04, Yoichi Nakayama via GitGitGadget wrote:
> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> 
> It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"
> 
> Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> ---
>   contrib/git-jump/git-jump | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index cc97b0dcf02..316e9628725 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -23,7 +23,22 @@ EOF
>   
>   open_editor() {
>   	editor=`git var GIT_EDITOR`
> -	eval "$editor -q \$1"
> +	case "$editor" in
> +	*emacs*)
> +		# Supported editor values are:
> +		# - emacs
> +		# - emacsclient
> +		# - emacsclient -t
> +		#
> +		# Wait for completion of the asynchronously executed process
> +		# to avoid race conditions in case of "emacsclient".
> +		eval "$editor --eval \"(prog1 (pop-to-buffer (compilation-start \\\"cat $@\\\" 'grep-mode)) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""

I've just tried this out and it is much nicer than v4, thank you for 
tweaking it. It is a little sluggish to pop up the emacs window though - 
are you sure we need the while loop? I've commented it out and it seems 
to work just fine. The documentation for pop-to-buffer says it selects 
the frame displaying the buffer so I don't think we need to wait before 
calling select-frame-set-input-focus (I'm no emacs expert though). I do 
think it would be better to quote the filename or better still call 
git-jump from compilation-start as Peff suggested. It would also be nice 
to stop emacsclient from printing anything in the terminal.

It would be nice to be able to run git-jump from within emacs. I came up 
with the code below which prompts the user for the directory to run 
git-jump in (which only matters for grep and diff --relative I think) 
and then checks for modified buffers visiting files in that repository 
before running git-jump.

Best Wishes

Phillip

---- >8 ----
(require 'cl-lib)

(defun git-jump (dir)
   "Run 'git jump', prompts for the directory to run in. Also prompts to
    save modified buffers visiting files in the repository containing DIR"
   (interactive "DDirectory:")
   (let* ((dir (expand-file-name dir))
	 (worktree (git-jump--get-worktree dir)))
     (unless worktree
       (error "Not in a git repository"))
     (git-jump--save-worktree-buffers worktree)
     ;; Use "cd" rather than "git -C" so emacs can tell which directory
     ;; the command is being run in.
     (compilation-start (concat "cd " (shell-quote-argument dir)
			       " && git jump --stdout "
			       (read-string "Jump command: "))
		       'grep-mode
		       (lambda (mode) "*git-jump*"))))


(defun git-jump--save-worktree-buffers (worktree)
   "Prompt the user to save all the modified buffers in WORKTREE"
   (let ((ht (make-hash-table :test 'equal))
	(off (length worktree))
	(buffers nil))
     (dolist (b (buffer-list))
       (when (buffer-modified-p b)
	(let ((file (buffer-file-name b)))
	  (when file
	    (let ((path (file-truename file)))
	      (when (string-prefix-p worktree path)
		(puthash (substring path off) b ht)))))))
     (let ((modified (hash-table-keys ht)))
       (when modified
	(git-jump--ls-files worktree
			       modified
			       (lambda (path)
				 (push (gethash path ht) buffers)))
	(when buffers
	  (save-some-buffers nil (lambda ()
				   (memq (current-buffer) buffers))))))))


(defun git-jump--get-worktree (dir)
   "Get the git worktree containing DIR. Returns nil if DIR is not in a
    repository"
   (message (concat "dir: " dir))
   (let* ((toplevel "")
	    (filter (lambda (_proc text)
		      (setf toplevel (concat toplevel text))))
	    (proc (make-process :name "rev-parse--toplevel"
				:buffer nil
				:coding (or file-name-coding-system
					    default-file-name-coding-system)
				:command (list "git" "-C" dir "rev-parse"
					       "--show-toplevel")
				:connection-type 'pipe
				:filter filter)))
        (while (or (accept-process-output proc 120)
		  (not (memq (process-status proc) '(exit signal)))))
        (prog1
	   (if (and (eq (process-status proc) 'exit)
		    (zerop (process-exit-status proc)))
	       (concat (substring toplevel 0 -1) "/")
	     nil)
	 (delete-process proc))))


(defun git-jump--ls-files (worktree paths func)
   "Run FUNC on PATHS that are tracked by worktree. NB takes paths not 
pathspecs"
   (let* ((remainder "")
	 (filter (lambda (_proc text)
		   (let* ((text (concat remainder text))
			  (len (length text))
			  (pos 0))
		     (while (< pos len)
		       (cond
			((= pos (string-match "\\([^\0]+\\)\0" text pos))
			 (funcall func (match-string 1 text))
			 (setq pos (match-end 0)))
			(t
			 (setq remainder (substring text pos))
			 (setq pos len)))))))
        (proc (make-process :name "ls-files"
			   :buffer nil
			   :coding (or file-name-coding-system
				       default-file-name-coding-system)
			   :command (cl-list* "git" "-C" worktree
					      "--literal-pathspecs" "ls-files"
					      "-z" "--" paths)
			   :connection-type 'pipe
			   :filter filter)))
     (while (or (accept-process-output proc 120)
	       (not (memq (process-status proc) '(exit signal)))))
     (delete-process proc)))

(provide 'git-jump)

Re: [PATCH v5 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Wed, Nov 23, 2022 at 11:58 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> I've just tried this out and it is much nicer than v4, thank you for
> tweaking it. It is a little sluggish to pop up the emacs window though -
> are you sure we need the while loop? I've commented it out and it seems
> to work just fine. The documentation for pop-to-buffer says it selects
> the frame displaying the buffer so I don't think we need to wait before
> calling select-frame-set-input-focus (I'm no emacs expert though). I do
> think it would be better to quote the filename or better still call
> git-jump from compilation-start as Peff suggested. It would also be nice
> to stop emacsclient from printing anything in the terminal.

As I wrote in the code comment, there is a race condition in
editor="emacsclient" case.
You can observe it by removing the while loop and insert "sleep 1 &&"
before cat command.
However, the while loop can be moved to the end, which reduces latency.
So, I'll change its position.

In editor="emacsclient" case, we can stop emacsclient from printing
anything in the
terminal by redirecting stdout to /dev/null. But it causes following error with
editor="emacsclient -t" case:
     emacsclient: could not get terminal name
You can use editor="emacsclient -u" to suppress output.

Best Wishes,
-- 
Yoichi NAKAYAMA

Re: [PATCH v5 0/3] git-jump: support Emacs

[Jeff King]

On Wed, Nov 23, 2022 at 07:04:10AM +0000, Yoichi NAKAYAMA via GitGitGadget wrote:

> Add an optional argument 'stdout' to print the quickfix lines to standard
> output. It can be used with M-x grep on Emacs.
> 
> Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
> EDITOR="emacsclient" and EDITOR="emacsclient -t".

Thanks, this version looks fine to me. I see your response to Phillip
may result in a v6 where the loop within the emacs code is moved, but I
have no opinion on that. :) So assuming that is the only change, my
review will still stand.

-Peff

[PATCH v6 0/3] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Jeff King (1):
  git-jump: move valid-mode check earlier

Yoichi Nakayama (2):
  git-jump: add an optional argument '--stdout'
  git-jump: invoke emacs/emacsclient

 contrib/git-jump/README   | 10 ++++++++-
 contrib/git-jump/git-jump | 45 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v5:

 1:  446777d300d = 1:  446777d300d git-jump: add an optional argument '--stdout'
 2:  972d51888ba = 2:  972d51888ba git-jump: move valid-mode check earlier
 3:  ad7c299cb0f ! 3:  446d39f62fb git-jump: invoke emacs/emacsclient
     @@ contrib/git-jump/git-jump: EOF
      +		#
      +		# Wait for completion of the asynchronously executed process
      +		# to avoid race conditions in case of "emacsclient".
     -+		eval "$editor --eval \"(prog1 (pop-to-buffer (compilation-start \\\"cat $@\\\" 'grep-mode)) (while (get-buffer-process (current-buffer)) (sleep-for 0.1)) (select-frame-set-input-focus (selected-frame)))\""
     ++		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat $@\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
      +		;;
      +	*)
      +		# assume anything else is vi-compatible

-- 
gitgitgadget

[PATCH v6 1/3] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..3211841305f 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,14 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument '--stdout' to print the listing to
+standard output instead of feeding it to the editor. You can use the
+argument with M-x grep on Emacs:
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
+M-x grep<RET>git jump --stdout diff<RET>
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +108,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..babb3b5c68d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [--stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `--stdout` is given, print the quickfix
+lines to standard output instead of feeding it to the editor.
 EOF
 }
 
@@ -64,11 +67,31 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+use_stdout=
+while test $# -gt 0; do
+	case "$1" in
+	--stdout)
+		use_stdout=t
+		shift
+		;;
+	--*)
+		usage >&2
+		exit 1
+		;;
+	*)
+		break
+		;;
+	esac
+done
 if test $# -lt 1; then
 	usage >&2
 	exit 1
 fi
 mode=$1; shift
+if test "$use_stdout" = "t"; then
+	"mode_$mode" "$@"
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

[PATCH v6 2/3] git-jump: move valid-mode check earlier

[Jeff King via GitGitGadget]

From: Jeff King <peff@peff.net>

We check if the "mode" argument supplied by the user is valid by seeing
if we have a mode_$mode function defined. But we don't do that until
after creating the tempfile. This is wasteful (we create a tempfile but
never use it), and makes it harder to add new options (the recent stdout
option exits before creating the tempfile, so it misses the check and
"git jump --stdout foo" will produce "git-jump: 92: mode_foo: not found"
rather than the regular usage message).

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/git-jump/git-jump | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index babb3b5c68d..cc97b0dcf02 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -88,6 +88,8 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+
 if test "$use_stdout" = "t"; then
 	"mode_$mode" "$@"
 	exit 0
@@ -95,7 +97,6 @@ fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
 open_editor "$tmp"
-- 
gitgitgadget

[PATCH v6 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index cc97b0dcf02..3e3911b1f9d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -23,7 +23,22 @@ EOF
 
 open_editor() {
 	editor=`git var GIT_EDITOR`
-	eval "$editor -q \$1"
+	case "$editor" in
+	*emacs*)
+		# Supported editor values are:
+		# - emacs
+		# - emacsclient
+		# - emacsclient -t
+		#
+		# Wait for completion of the asynchronously executed process
+		# to avoid race conditions in case of "emacsclient".
+		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat $@\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
+		;;
+	*)
+		# assume anything else is vi-compatible
+		eval "$editor -q \$1"
+		;;
+	esac
 }
 
 mode_diff() {
-- 
gitgitgadget

[PATCH v7 0/3] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Jeff King (1):
  git-jump: move valid-mode check earlier

Yoichi Nakayama (2):
  git-jump: add an optional argument '--stdout'
  git-jump: invoke emacs/emacsclient

 contrib/git-jump/README   | 10 ++++++++-
 contrib/git-jump/git-jump | 45 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v7
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v6:

 1:  446777d300d = 1:  446777d300d git-jump: add an optional argument '--stdout'
 2:  972d51888ba = 2:  972d51888ba git-jump: move valid-mode check earlier
 3:  446d39f62fb ! 3:  d8233f96175 git-jump: invoke emacs/emacsclient
     @@ contrib/git-jump/git-jump: EOF
      +		#
      +		# Wait for completion of the asynchronously executed process
      +		# to avoid race conditions in case of "emacsclient".
     -+		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat $@\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
     ++		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat \$1\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
      +		;;
      +	*)
      +		# assume anything else is vi-compatible

-- 
gitgitgadget

[PATCH v7 1/3] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..3211841305f 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,14 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument '--stdout' to print the listing to
+standard output instead of feeding it to the editor. You can use the
+argument with M-x grep on Emacs:
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
+M-x grep<RET>git jump --stdout diff<RET>
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +108,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..babb3b5c68d 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [--stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `--stdout` is given, print the quickfix
+lines to standard output instead of feeding it to the editor.
 EOF
 }
 
@@ -64,11 +67,31 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+use_stdout=
+while test $# -gt 0; do
+	case "$1" in
+	--stdout)
+		use_stdout=t
+		shift
+		;;
+	--*)
+		usage >&2
+		exit 1
+		;;
+	*)
+		break
+		;;
+	esac
+done
 if test $# -lt 1; then
 	usage >&2
 	exit 1
 fi
 mode=$1; shift
+if test "$use_stdout" = "t"; then
+	"mode_$mode" "$@"
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

Re: [PATCH v7 1/3] git-jump: add an optional argument '--stdout'

[Ævar Arnfjörð Bjarmason]

On Fri, Nov 25 2022, Yoichi Nakayama via GitGitGadget wrote:

> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

> +use_stdout=
> +while test $# -gt 0; do
> +	case "$1" in
> +	--stdout)
> +		use_stdout=t
> +		shift

Put this "shift"....

> +		;;
> +	--*)
> +		usage >&2
> +		exit 1
> +		;;
> +	*)
> +		break
> +		;;
> +	esac

... here instead, as I showed in a comment on 3/3 you'd need to to that,
or copy/paste it for every option once you have >1 option. See
e.g. "test_commit" in "t/test-lib-functions.sh" for a function with that
pattern.

Re: [PATCH v7 1/3] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama]

On Fri, Nov 25, 2022 at 6:10 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Fri, Nov 25 2022, Yoichi Nakayama via GitGitGadget wrote:
>
> > From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
>
> > +use_stdout=
> > +while test $# -gt 0; do
> > +     case "$1" in
> > +     --stdout)
> > +             use_stdout=t
> > +             shift
>
> Put this "shift"....
>
> > +             ;;
> > +     --*)
> > +             usage >&2
> > +             exit 1
> > +             ;;
> > +     *)
> > +             break
> > +             ;;
> > +     esac
>
> ... here instead, as I showed in a comment on 3/3 you'd need to to that,
> or copy/paste it for every option once you have >1 option. See
> e.g. "test_commit" in "t/test-lib-functions.sh" for a function with that
> pattern.

Thanks. I'll apply this.

-- 
Yoichi NAKAYAMA

[PATCH v7 2/3] git-jump: move valid-mode check earlier

[Jeff King via GitGitGadget]

From: Jeff King <peff@peff.net>

We check if the "mode" argument supplied by the user is valid by seeing
if we have a mode_$mode function defined. But we don't do that until
after creating the tempfile. This is wasteful (we create a tempfile but
never use it), and makes it harder to add new options (the recent stdout
option exits before creating the tempfile, so it misses the check and
"git jump --stdout foo" will produce "git-jump: 92: mode_foo: not found"
rather than the regular usage message).

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/git-jump/git-jump | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index babb3b5c68d..cc97b0dcf02 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -88,6 +88,8 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+
 if test "$use_stdout" = "t"; then
 	"mode_$mode" "$@"
 	exit 0
@@ -95,7 +97,6 @@ fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
 open_editor "$tmp"
-- 
gitgitgadget

[PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index cc97b0dcf02..eef9cda832f 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -23,7 +23,22 @@ EOF
 
 open_editor() {
 	editor=`git var GIT_EDITOR`
-	eval "$editor -q \$1"
+	case "$editor" in
+	*emacs*)
+		# Supported editor values are:
+		# - emacs
+		# - emacsclient
+		# - emacsclient -t
+		#
+		# Wait for completion of the asynchronously executed process
+		# to avoid race conditions in case of "emacsclient".
+		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat \$1\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
+		;;
+	*)
+		# assume anything else is vi-compatible
+		eval "$editor -q \$1"
+		;;
+	esac
 }
 
 mode_diff() {
-- 
gitgitgadget

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Ævar Arnfjörð Bjarmason]

On Fri, Nov 25 2022, Yoichi Nakayama via GitGitGadget wrote:

> From: Yoichi Nakayama <yoichi.nakayama@gmail.com>
>
> It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"
>
> Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
> ---
>  contrib/git-jump/git-jump | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index cc97b0dcf02..eef9cda832f 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -23,7 +23,22 @@ EOF
>  
>  open_editor() {
>  	editor=`git var GIT_EDITOR`
> -	eval "$editor -q \$1"
> +	case "$editor" in
> +	*emacs*)
> +		# Supported editor values are:
> +		# - emacs
> +		# - emacsclient
> +		# - emacsclient -t
> +		#
> +		# Wait for completion of the asynchronously executed process
> +		# to avoid race conditions in case of "emacsclient".
> +		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat \$1\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
> +		;;
> +	*)
> +		# assume anything else is vi-compatible
> +		eval "$editor -q \$1"
> +		;;
> +	esac
>  }
>  
>  mode_diff() {

I'd really like to have some closer and smarter emacs integration like
this.

But I don't see why we need to run the grep ourselves, pipe it to a
temporary file, and then discover that we're using emacs, and --eval
code into it to switch to that buffer, and fake up a "M-x grep" command
with a compilation buffer to make it look like we ran M-x grep in the
first place.

Let's just ... run M-x grep earlier? Then we can skip all the earlier
steps.

I experimented with this a bit locally, and I didn't get the "switch to
buffer" semantics to work with this (but that's presumably easy, I'm
just rusty on my elisp APIs), but something in this direction seems much
better:
	
	diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
	index eef9cda832f..c932d7acd0f 100755
	--- a/contrib/git-jump/git-jump
	+++ b/contrib/git-jump/git-jump
	@@ -22,23 +22,7 @@ EOF
	 }
	 
	 open_editor() {
	-	editor=`git var GIT_EDITOR`
	-	case "$editor" in
	-	*emacs*)
	-		# Supported editor values are:
	-		# - emacs
	-		# - emacsclient
	-		# - emacsclient -t
	-		#
	-		# Wait for completion of the asynchronously executed process
	-		# to avoid race conditions in case of "emacsclient".
	-		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat \$1\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
	-		;;
	-	*)
	-		# assume anything else is vi-compatible
	-		eval "$editor -q \$1"
	-		;;
	-	esac
	+	eval "$editor -q \$1"
	 }
	 
	 mode_diff() {
	@@ -83,11 +67,14 @@ mode_ws() {
	 }
	 
	 use_stdout=
	+use_magic=t
	 while test $# -gt 0; do
	 	case "$1" in
	 	--stdout)
	 		use_stdout=t
	-		shift
	+		;;
	+	--no-magic)
	+		use_magic=
	 		;;
	 	--*)
	 		usage >&2
	@@ -96,7 +83,8 @@ while test $# -gt 0; do
	 	*)
	 		break
	 		;;
	-	esac
	+	esac &&
	+	shift
	 done
	 if test $# -lt 1; then
	 	usage >&2
	@@ -105,6 +93,22 @@ fi
	 mode=$1; shift
	 type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
	 
	+editor=`git var GIT_EDITOR`
	+if test "$use_magic" && test "$mode" = "grep"
	+then
	+	case "$editor" in
	+	*emacs*)
	+		set -x
	+		eval "$editor --eval \" \
	+			(grep \\\"git grep -H "$@"\\\") \
	+		\""
	+		exit
	+		;;
	+	*)
	+		;;
	+	esac
	+fi
	+
	 if test "$use_stdout" = "t"; then
	 	"mode_$mode" "$@"
	 	exit 0

I.e. if we're going to trust emacs to eval this code, and assume that
grep.el etc. works, let's just run the equivalent of M-x grep with our
'git grep' command.

This is already better in that "grep" understands that I searched for
"foo.*bar", so that's highlighted in the resulting buffer, just as with
normal "grep" commands.

This is missing the bit where we'd need to jump.grepCmd etc, so it's
incomplete.

I think this is all the prior art we'd need to invoke "git grep" the
right way from emacs's "grep":
https://github.com/eglaysher/find-things-fast/blob/master/find-things-fast.el#L246

B.t.w. I'd think the "--no-magic" here is something you'd want too. I
like this new behavior (sans the comments above), but presumably there's
people using emacs as their EDITOR who don't want to have this magic
behavior, having an opt-out would be neat.

I.e. if you have an existing customization intercepting these then this
will screw with that, but maybe that's OK...

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Fri, Nov 25, 2022 at 6:08 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> I'd really like to have some closer and smarter emacs integration like
> this.
>
> But I don't see why we need to run the grep ourselves, pipe it to a
> temporary file, and then discover that we're using emacs, and --eval
> code into it to switch to that buffer, and fake up a "M-x grep" command
> with a compilation buffer to make it look like we ran M-x grep in the
> first place.
>
> Let's just ... run M-x grep earlier? Then we can skip all the earlier
> steps.

There are two reasons.

First, I want to reuse the modes that git-jump already have. In
addition to mode_grep,
mode_{diff,merge,ws} exist, and if we re-implement each for editor
support, I think it will be
difficult to maintain.

Second, there is a difficulty passing arbitrary arguments properly to
Emacs Lisp properly.
For example, your version will cause error with
        git jump grep "hello world"
My early patch was doing something similar. But the second problem was
hard to deal with,
so I switched to using a temporary file.
-- 
Yoichi NAKAYAMA

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Ævar Arnfjörð Bjarmason]

On Sat, Nov 26 2022, Yoichi Nakayama wrote:

> On Fri, Nov 25, 2022 at 6:08 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> I'd really like to have some closer and smarter emacs integration like
>> this.
>>
>> But I don't see why we need to run the grep ourselves, pipe it to a
>> temporary file, and then discover that we're using emacs, and --eval
>> code into it to switch to that buffer, and fake up a "M-x grep" command
>> with a compilation buffer to make it look like we ran M-x grep in the
>> first place.
>>
>> Let's just ... run M-x grep earlier? Then we can skip all the earlier
>> steps.
>
> There are two reasons.
>
> First, I want to reuse the modes that git-jump already have. In
> addition to mode_grep,
> mode_{diff,merge,ws} exist, and if we re-implement each for editor
> support, I think it will be
> difficult to maintain.

Yeah, maybe that'll be painful. I haven't poked much at it...

> Second, there is a difficulty passing arbitrary arguments properly to
> Emacs Lisp properly.
> For example, your version will cause error with
>         git jump grep "hello world"
> My early patch was doing something similar. But the second problem was
> hard to deal with,
> so I switched to using a temporary file.

To the extent that that's painful couldn't we write the grep expression
/ arguments to the tempfile, then feed the tempfile to the ad-hoc elisp
code?

It would then read it, get the argument to grep for, and we'd call (grep
that-argument).

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Jeff King]

On Sat, Nov 26, 2022 at 12:52:50AM +0100, Ævar Arnfjörð Bjarmason wrote:

> > Second, there is a difficulty passing arbitrary arguments properly to
> > Emacs Lisp properly.
> > For example, your version will cause error with
> >         git jump grep "hello world"
> > My early patch was doing something similar. But the second problem was
> > hard to deal with,
> > so I switched to using a temporary file.
> 
> To the extent that that's painful couldn't we write the grep expression
> / arguments to the tempfile, then feed the tempfile to the ad-hoc elisp
> code?
> 
> It would then read it, get the argument to grep for, and we'd call (grep
> that-argument).

You'd still need to quote the arguments, since you'll be reading
potentially multiple arguments out of the bytestream of the file[1].

If you're not going to quote, the simplest thing is to generate the
line-oriented output and read that.

If you are going to quote, then you don't need the tempfile at all. You
can shove the command into the eval, as if git-jump were run from emacs
directly (but you want to use the --stdout mode introduced in this
series, and not the git commands directly, because of course they're
non-trivial).

I showed how to do the quoting earlier in the thread. But it is ugly,
and this tempfile hack should work (modulo the gross wait loop
afterwards).

-Peff

[1] Of course you could have a stripped-down version that only greps and
    only takes one argument, but then why are you using git-jump in the
    first place?

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 28 2022, Jeff King wrote:

> On Sat, Nov 26, 2022 at 12:52:50AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > Second, there is a difficulty passing arbitrary arguments properly to
>> > Emacs Lisp properly.
>> > For example, your version will cause error with
>> >         git jump grep "hello world"
>> > My early patch was doing something similar. But the second problem was
>> > hard to deal with,
>> > so I switched to using a temporary file.
>> 
>> To the extent that that's painful couldn't we write the grep expression
>> / arguments to the tempfile, then feed the tempfile to the ad-hoc elisp
>> code?
>> 
>> It would then read it, get the argument to grep for, and we'd call (grep
>> that-argument).
>
> You'd still need to quote the arguments, since you'll be reading
> potentially multiple arguments out of the bytestream of the file[1].
>
> If you're not going to quote, the simplest thing is to generate the
> line-oriented output and read that.
>
> If you are going to quote, then you don't need the tempfile at all. You
> can shove the command into the eval, as if git-jump were run from emacs
> directly (but you want to use the --stdout mode introduced in this
> series, and not the git commands directly, because of course they're
> non-trivial).
>
> I showed how to do the quoting earlier in the thread. But it is ugly,
> and this tempfile hack should work (modulo the gross wait loop
> afterwards).

Thanks, I'd missed
https://lore.kernel.org/git/Y30a0ulfxyE7dnYi@coredump.intra.peff.net/

I think the case where the temporary directory itself has spaces in it
isn't worth worrying about.

So, all we'd need to worry about is getting the arguments to be grep'd
to emacs.

That should be simpler & bug-free with some equivalent of

     echo "args" >$tmpfile

then in Emacs, given some "<tmpfile>" variable:

  (with-temp-buffer
    (insert-file-contents <tempfile>)
    (buffer-string)))

We'd then invoke M-x grep with that.

I think getting rid of the tempfile isn't worth it, or worth worrying
about, what I was pointing out is that the implementation as it stands
works notably differently than if you invoked M-x grep itself.

I.e. it doesn't do highlighting, and (I didn't note this before) if it
takes a while we'll "hang", if we had emacs itself invoke the "git grep"
we'd stream out grep results as they came in.

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Mon, Nov 28, 2022 at 8:03 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> I think getting rid of the tempfile isn't worth it, or worth worrying
> about, what I was pointing out is that the implementation as it stands
> works notably differently than if you invoked M-x grep itself.
>
> I.e. it doesn't do highlighting, and (I didn't note this before) if it
> takes a while we'll "hang", if we had emacs itself invoke the "git grep"
> we'd stream out grep results as they came in.

In PATCH v8, we stopped imitating the grep function and
changed it to call the grep function.

About the first problem,
I still don't understand what you mean by highlighting problem.
On my environment, the "*grep*" buffer is colored
(i.e. filnemame:line_number part on each line is colored).

About the second problem,
If the "hang" is until writing to tempfile is completed, it is unavoidable
as long as tempfile is used.
If the "hang" is no response from Emacs when "cat tempfile" takes a
long time, we can reduce the duration by detecting a change in
(point-max) and exiting the loop.

--
Yoichi NAKAYAMA

Re: [PATCH v7 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama]

On Sat, Nov 26, 2022 at 1:01 AM Yoichi Nakayama
<yoichi.nakayama@gmail.com> wrote:
> Second, there is a difficulty passing arbitrary arguments properly to
> Emacs Lisp properly.
> For example, your version will cause error with
>         git jump grep "hello world"
> My early patch was doing something similar. But the second problem was
> hard to deal with,
> so I switched to using a temporary file.

But even in that case it was fine to use the original grep function
defined in grep.el.
I'll fix it in v8.


--
Yoichi NAKAYAMA

[PATCH v8 0/3] git-jump: support Emacs

[Yoichi NAKAYAMA via GitGitGadget]

Add an optional argument 'stdout' to print the quickfix lines to standard
output. It can be used with M-x grep on Emacs.

Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
EDITOR="emacsclient" and EDITOR="emacsclient -t".

Jeff King (1):
  git-jump: move valid-mode check earlier

Yoichi Nakayama (2):
  git-jump: add an optional argument '--stdout'
  git-jump: invoke emacs/emacsclient

 contrib/git-jump/README   | 10 ++++++++-
 contrib/git-jump/git-jump | 45 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)


base-commit: eea7033409a0ed713c78437fc76486983d211e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v8
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v8
Pull-Request: https://github.com/gitgitgadget/git/pull/1423

Range-diff vs v7:

 1:  446777d300d ! 1:  afface9b010 git-jump: add an optional argument '--stdout'
     @@ contrib/git-jump/git-jump: mode_ws() {
      +	case "$1" in
      +	--stdout)
      +		use_stdout=t
     -+		shift
      +		;;
      +	--*)
      +		usage >&2
     @@ contrib/git-jump/git-jump: mode_ws() {
      +		break
      +		;;
      +	esac
     ++	shift
      +done
       if test $# -lt 1; then
       	usage >&2
 2:  972d51888ba = 2:  e9aa6fdf836 git-jump: move valid-mode check earlier
 3:  d8233f96175 ! 3:  048f508ca99 git-jump: invoke emacs/emacsclient
     @@ contrib/git-jump/git-jump: EOF
      +		#
      +		# Wait for completion of the asynchronously executed process
      +		# to avoid race conditions in case of "emacsclient".
     -+		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat \$1\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
     ++		eval "$editor --eval \"(let ((buf (grep \\\"cat \$1\\\"))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
      +		;;
      +	*)
      +		# assume anything else is vi-compatible

-- 
gitgitgadget

[PATCH v8 1/3] git-jump: add an optional argument '--stdout'

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It can be used with M-x grep on Emacs.

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/README   | 10 +++++++++-
 contrib/git-jump/git-jump | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
index 8bcace29d21..3211841305f 100644
--- a/contrib/git-jump/README
+++ b/contrib/git-jump/README
@@ -79,6 +79,14 @@ git jump grep -i foo_bar
 git config jump.grepCmd "ag --column"
 --------------------------------------------------
 
+You can use the optional argument '--stdout' to print the listing to
+standard output instead of feeding it to the editor. You can use the
+argument with M-x grep on Emacs:
+
+--------------------------------------------------
+# In Emacs, M-x grep and invoke "git jump --stdout <mode>"
+M-x grep<RET>git jump --stdout diff<RET>
+--------------------------------------------------
 
 Related Programs
 ----------------
@@ -100,7 +108,7 @@ Limitations
 -----------
 
 This script was written and tested with vim. Given that the quickfix
-format is the same as what gcc produces, I expect emacs users have a
+format is the same as what gcc produces, I expect other tools have a
 similar feature for iterating through the list, but I know nothing about
 how to activate it.
 
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 92dbd4cde18..be0642bbe34 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -2,7 +2,7 @@
 
 usage() {
 	cat <<\EOF
-usage: git jump <mode> [<args>]
+usage: git jump [--stdout] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -15,6 +15,9 @@ grep: elements are grep hits. Arguments are given to git grep or, if
       configured, to the command in `jump.grepCmd`.
 
 ws: elements are whitespace errors. Arguments are given to diff --check.
+
+If the optional argument `--stdout` is given, print the quickfix
+lines to standard output instead of feeding it to the editor.
 EOF
 }
 
@@ -64,11 +67,31 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+use_stdout=
+while test $# -gt 0; do
+	case "$1" in
+	--stdout)
+		use_stdout=t
+		;;
+	--*)
+		usage >&2
+		exit 1
+		;;
+	*)
+		break
+		;;
+	esac
+	shift
+done
 if test $# -lt 1; then
 	usage >&2
 	exit 1
 fi
 mode=$1; shift
+if test "$use_stdout" = "t"; then
+	"mode_$mode" "$@"
+	exit 0
+fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-- 
gitgitgadget

[PATCH v8 2/3] git-jump: move valid-mode check earlier

[Jeff King via GitGitGadget]

From: Jeff King <peff@peff.net>

We check if the "mode" argument supplied by the user is valid by seeing
if we have a mode_$mode function defined. But we don't do that until
after creating the tempfile. This is wasteful (we create a tempfile but
never use it), and makes it harder to add new options (the recent stdout
option exits before creating the tempfile, so it misses the check and
"git jump --stdout foo" will produce "git-jump: 92: mode_foo: not found"
rather than the regular usage message).

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/git-jump/git-jump | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index be0642bbe34..a5a8a77e20e 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -88,6 +88,8 @@ if test $# -lt 1; then
 	exit 1
 fi
 mode=$1; shift
+type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
+
 if test "$use_stdout" = "t"; then
 	"mode_$mode" "$@"
 	exit 0
@@ -95,7 +97,6 @@ fi
 
 trap 'rm -f "$tmp"' 0 1 2 3 15
 tmp=`mktemp -t git-jump.XXXXXX` || exit 1
-type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
 "mode_$mode" "$@" >"$tmp"
 test -s "$tmp" || exit 0
 open_editor "$tmp"
-- 
gitgitgadget

[PATCH v8 3/3] git-jump: invoke emacs/emacsclient

[Yoichi Nakayama via GitGitGadget]

From: Yoichi Nakayama <yoichi.nakayama@gmail.com>

It works with GIT_EDITOR="emacs", "emacsclient" or "emacsclient -t"

Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com>
---
 contrib/git-jump/git-jump | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index a5a8a77e20e..40c4b0d1110 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -23,7 +23,22 @@ EOF
 
 open_editor() {
 	editor=`git var GIT_EDITOR`
-	eval "$editor -q \$1"
+	case "$editor" in
+	*emacs*)
+		# Supported editor values are:
+		# - emacs
+		# - emacsclient
+		# - emacsclient -t
+		#
+		# Wait for completion of the asynchronously executed process
+		# to avoid race conditions in case of "emacsclient".
+		eval "$editor --eval \"(let ((buf (grep \\\"cat \$1\\\"))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
+		;;
+	*)
+		# assume anything else is vi-compatible
+		eval "$editor -q \$1"
+		;;
+	esac
 }
 
 mode_diff() {
-- 
gitgitgadget

Re: [PATCH v8 0/3] git-jump: support Emacs

[Phillip Wood]

Hi Yoichi

On 27/11/2022 01:18, Yoichi NAKAYAMA via GitGitGadget wrote:
> Add an optional argument 'stdout' to print the quickfix lines to standard
> output. It can be used with M-x grep on Emacs.
> 
> Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
> EDITOR="emacsclient" and EDITOR="emacsclient -t".

I've tested this version and it addresses all of my previous concerns, 
thanks for working on it.

Best Wishes

Phillip

> Jeff King (1):
>    git-jump: move valid-mode check earlier
> 
> Yoichi Nakayama (2):
>    git-jump: add an optional argument '--stdout'
>    git-jump: invoke emacs/emacsclient
> 
>   contrib/git-jump/README   | 10 ++++++++-
>   contrib/git-jump/git-jump | 45 ++++++++++++++++++++++++++++++++++++---
>   2 files changed, 51 insertions(+), 4 deletions(-)
> 
> 
> base-commit: eea7033409a0ed713c78437fc76486983d211e25
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1423%2Fyoichi%2Fgit-jump-emacs-support-v8
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1423/yoichi/git-jump-emacs-support-v8
> Pull-Request: https://github.com/gitgitgadget/git/pull/1423
> 
> Range-diff vs v7:
> 
>   1:  446777d300d ! 1:  afface9b010 git-jump: add an optional argument '--stdout'
>       @@ contrib/git-jump/git-jump: mode_ws() {
>        +	case "$1" in
>        +	--stdout)
>        +		use_stdout=t
>       -+		shift
>        +		;;
>        +	--*)
>        +		usage >&2
>       @@ contrib/git-jump/git-jump: mode_ws() {
>        +		break
>        +		;;
>        +	esac
>       ++	shift
>        +done
>         if test $# -lt 1; then
>         	usage >&2
>   2:  972d51888ba = 2:  e9aa6fdf836 git-jump: move valid-mode check earlier
>   3:  d8233f96175 ! 3:  048f508ca99 git-jump: invoke emacs/emacsclient
>       @@ contrib/git-jump/git-jump: EOF
>        +		#
>        +		# Wait for completion of the asynchronously executed process
>        +		# to avoid race conditions in case of "emacsclient".
>       -+		eval "$editor --eval \"(let ((buf (compilation-start \\\"cat \$1\\\" 'grep-mode))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
>       ++		eval "$editor --eval \"(let ((buf (grep \\\"cat \$1\\\"))) (pop-to-buffer buf) (select-frame-set-input-focus (selected-frame)) (while (get-buffer-process buf) (sleep-for 0.1)))\""
>        +		;;
>        +	*)
>        +		# assume anything else is vi-compatible
> 

Re: [PATCH v8 0/3] git-jump: support Emacs

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Yoichi
>
> On 27/11/2022 01:18, Yoichi NAKAYAMA via GitGitGadget wrote:
>> Add an optional argument 'stdout' to print the quickfix lines to standard
>> output. It can be used with M-x grep on Emacs.
>> Detect emacsclient by GIT_EDITOR and invoke the function. Tested
>> with
>> EDITOR="emacsclient" and EDITOR="emacsclient -t".
>
> I've tested this version and it addresses all of my previous concerns,
> thanks for working on it.

Thanks.

Re: [PATCH v8 0/3] git-jump: support Emacs

[Yoichi Nakayama]

Hi Phillip

On Mon, Nov 28, 2022 at 7:13 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Yoichi
>
> On 27/11/2022 01:18, Yoichi NAKAYAMA via GitGitGadget wrote:
> > Add an optional argument 'stdout' to print the quickfix lines to standard
> > output. It can be used with M-x grep on Emacs.
> >
> > Detect emacsclient by GIT_EDITOR and invoke the function. Tested with
> > EDITOR="emacsclient" and EDITOR="emacsclient -t".
>
> I've tested this version and it addresses all of my previous concerns,
> thanks for working on it.

Thank you for confirming.
Your feedback has allowed us to refine the implementation.

-- 
Yoichi NAKAYAMA