[PATCH] contrib/git-jump: cat output when not a terminal

[PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

contrib/git-jump: cat output when not a terminal

The current usage to populate Vim's quickfix list cannot be used from
within the editor as it invokes another instance.

Check if stdout is to a terminal or not. If not simply cat the output.

Signed-off-by: George Brown <321.george@gmail.com>
---
 contrib/git-jump/git-jump | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 931b0fe3a9..253341c64e 100755
--- a/contrib/git-jump/git-jump
+++ b/contrib/git-jump/git-jump
@@ -19,8 +19,12 @@ EOF
 }

 open_editor() {
-    editor=`git var GIT_EDITOR`
-    eval "$editor -q \$1"
+    if test -t 1; then
+        editor=`git var GIT_EDITOR`
+        eval "$editor -q \$1"
+    else
+        cat "$1"
+    fi
 }

 mode_diff() {

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

George Brown <321.george@gmail.com> writes:

> contrib/git-jump: cat output when not a terminal
>
> The current usage to populate Vim's quickfix list cannot be used from
> within the editor as it invokes another instance.
>
> Check if stdout is to a terminal or not. If not simply cat the output.

The are editors that open their own window, or tell an already open
editor instance to edit the buffer (think: emacsclient) and in order
to be functional, they do not have to be launched from inside a
working terminal window (whose input comes from the keyboard---the
test "-t 1" is an approximate check for that).

Wouldn't this change hurt the users of such an editor?

Would it work to set GIT_EDITOR to "cat" while performing the
"populating quickfix list" (whatever that is) operation?

> Signed-off-by: George Brown <321.george@gmail.com>
> ---
>  contrib/git-jump/git-jump | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index 931b0fe3a9..253341c64e 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -19,8 +19,12 @@ EOF
>  }
>
>  open_editor() {
> -    editor=`git var GIT_EDITOR`
> -    eval "$editor -q \$1"
> +    if test -t 1; then
> +        editor=`git var GIT_EDITOR`
> +        eval "$editor -q \$1"
> +    else
> +        cat "$1"
> +    fi
>  }
>
>  mode_diff() {

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

> The are editors that open their own window, or tell an already open
> editor instance to edit the buffer (think: emacsclient) and in order
> to be functional, they do not have to be launched from inside a
> working terminal window (whose input comes from the keyboard---the
> test "-t 1" is an approximate check for that).
>
> Wouldn't this change hurt the users of such an editor?

I'm unsure of emacsclient but I would have thought that editors in
general would be more amenable to receiving lines than relying on
additional spawned instances to behave as clients.

> Would it work to set GIT_EDITOR to "cat" while performing the
> "populating quickfix list" (whatever that is) operation?

It would, though I considered this to be less elegant.

Just for background the quickfix list in Vim is simply a list of
locations that you can browse and jump to.

On Sat, 9 May 2020 at 22:41, Junio C Hamano <gitster@pobox.com> wrote:
>
> George Brown <321.george@gmail.com> writes:
>
> > contrib/git-jump: cat output when not a terminal
> >
> > The current usage to populate Vim's quickfix list cannot be used from
> > within the editor as it invokes another instance.
> >
> > Check if stdout is to a terminal or not. If not simply cat the output.
>
> The are editors that open their own window, or tell an already open
> editor instance to edit the buffer (think: emacsclient) and in order
> to be functional, they do not have to be launched from inside a
> working terminal window (whose input comes from the keyboard---the
> test "-t 1" is an approximate check for that).
>
> Wouldn't this change hurt the users of such an editor?
>
> Would it work to set GIT_EDITOR to "cat" while performing the
> "populating quickfix list" (whatever that is) operation?
>
> > Signed-off-by: George Brown <321.george@gmail.com>
> > ---
> >  contrib/git-jump/git-jump | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> > index 931b0fe3a9..253341c64e 100755
> > --- a/contrib/git-jump/git-jump
> > +++ b/contrib/git-jump/git-jump
> > @@ -19,8 +19,12 @@ EOF
> >  }
> >
> >  open_editor() {
> > -    editor=`git var GIT_EDITOR`
> > -    eval "$editor -q \$1"
> > +    if test -t 1; then
> > +        editor=`git var GIT_EDITOR`
> > +        eval "$editor -q \$1"
> > +    else
> > +        cat "$1"
> > +    fi
> >  }
> >
> >  mode_diff() {

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

George Brown <321.george@gmail.com> writes:

>> The are editors that open their own window, or tell an already open
>> editor instance to edit the buffer (think: emacsclient) and in order
>> to be functional, they do not have to be launched from inside a
>> working terminal window (whose input comes from the keyboard---the
>> test "-t 1" is an approximate check for that).
>>
>> Wouldn't this change hurt the users of such an editor?
>
> I'm unsure of emacsclient but I would have thought that editors in
> general would be more amenable to receiving lines than relying on
> additional spawned instances to behave as clients.

I am not sure what you mean by "receiving lines".  Your patch makes
the "git jump" command completely ignore configured GIT_EDITOR and
refuse to invoke *any* editor.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

> I am not sure what you mean by "receiving lines".  Your patch makes
> the "git jump" command completely ignore configured GIT_EDITOR and
> refuse to invoke *any* editor.

Running "git jump" from a shell prompt works as before with this patch.

But now within Vim it can also be used to populate the quickfix list
with something like ":cexpr system('git jump')". Because instead of
invoking another instance of (which does not work well for Vim) it
just receives the lines as formatted by "git jump".

> > Would it work to set GIT_EDITOR to "cat" while performing the
> > "populating quickfix list" (whatever that is) operation?
>
> It would, though I considered this to be less elegant.

Just to revise this, it wouldn't actually as the editor is always passed
"-q" (intended for Vim's quickfix list). So overriding this to "cat"
causes as error as "-q" is not a valid option for it.

I'm aware the examples I'm giving are specific to Vim but it really
seems that "git jump" was intended with Vim in mind. That said I'd argue
most other editors would work better with "git jump" if when they
invoked it another instance wasn't created. I've tried find an
equivalent to Vim's quickfix in Emacs this morning but can't quite wrap
my head around it.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

George Brown <321.george@gmail.com> writes:

> Running "git jump" from a shell prompt works as before with this patch.

I am not worried at all about that use case.  It is clear that you
had the "shell prompt" in mind from "test -t 1".

Given what "git jump" does, wouldn't it be natural to expect that
GUI programs that have no "shell prompt" interaction with the end
user want to use "git jump"?  It may even be driving a graphical
version of vim---or is it impossible for such a GUI program to
exist?

As long as such a regression for existing users use is impossible, I
think the patch is probably OK, but doing a hardcoded "cat" smells
like a very bad hack, compared to a solution on the program's side
that *wants* to read the prepared file to arrange that to happen
(e.g. via setting the GIT_EDITOR environment to "cat" within that
program).

In any case, I am not a "git jump" user, so...

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

> As long as such a regression for existing users use is impossible, I
> think the patch is probably OK, but doing a hardcoded "cat" smells
> like a very bad hack, compared to a solution on the program's side
> that *wants* to read the prepared file to arrange that to happen
> (e.g. via setting the GIT_EDITOR environment to "cat" within that
> program).

I think this change only enhances "git jump".

> In any case, I am not a "git jump" user, so...

I looked into this as other Vim users were talking about it and wanted
this behavior.

https://www.reddit.com/r/vim/comments/gf2he2/what_is_the_simplest_way_to_get_all_git_changes/fprlxtk/?context=3

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

George Brown <321.george@gmail.com> writes:

>> As long as such a regression for existing users use is impossible, I
>> think the patch is probably OK, but doing a hardcoded "cat" smells
>> like a very bad hack, compared to a solution on the program's side
>> that *wants* to read the prepared file to arrange that to happen
>> (e.g. via setting the GIT_EDITOR environment to "cat" within that
>> program).
>
> I think this change only enhances "git jump".
>
>> In any case, I am not a "git jump" user, so...
>
> I looked into this as other Vim users were talking about it and wanted
> this behavior.

That's already irrelevant in the course of this discussion, no?

We have already established that you wrote in good faith to improve
the use experience by vim users, and nobody in this exchange doubts
that this change would help vim users.  

I am worried about the change hurting non-vim users, but no amount
of "me too" from vim users who care only about their vim experience
would allay that.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

I think with this change all editors can benefit. The format "git jump"
is producing is something easily consumed. I think consumption of output
from tools is far more common in editors than communication between
multiple instances.

As an aside the fact that as is "git jump" invokes "$GIT_EDITOR" with
the "-q" option makes an implicit assumption the editor will be Vim or
something very much like it. To be very clear I don't mean to say this
means only Vim should be considered. However it's also making the
implicit assumption that passing the "-q" option is valid for any
"$GIT_EDITOR" and does not cause an error like that seen when trying to
override "$GIT_EDITOR" with cat. This change means other editors can
invoke "git jump" without fear of such a situation, increasing
usability.

Arguably the most interoperable way for "git jump" to work would be to
output the formatted lines and do nothing else, leaving it to users to
choose how to operate upon the output/invoke editors. Of course such
a change would break the workflow of anyone who uses "git jump" today
and isn't a valid option.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

George Brown <321.george@gmail.com> writes:

> I think with this change all editors can benefit.

I am worried not only about the editors that are launched by
git-jump that runs GIT_EDITOR, or the use case where you run
git-jump from within your editor.  "git jump" that is launched from
a process, whose standard output steam is not connected to a
terminal, used to spawn the specified editor just fine.  Imagine a
user wanted to spawn a graphical editor via git-jump from within a
GUI script (probably launched from a window manager menu and has no
terminal).  With git-jump with this patch, such a use will be
broken, no?  The GIT_EDITOR the user set is totally ignored.

At the very least, we should have an escape hatch to help those this
patch is hurting, perhaps like

        open_editor() {
                if ! test -t 1 && test -z "$GIT_JUMP_IGNORE_STDOUT_CHECK"
                then
                        cat "$1"
                else
                        editor=`git var GIT_EDITOR`
                        eval "$editor -q \$1"
                fi
        }

so that at least they can spawn their chosen editor, not "cat", from
the tool they have been using.  

Because this is a new feature, instead of breaking existing users
and forcing them to do something different they did not have to
(namely, set and export the GIT_JUMP_IGNORE_STDOUT_CHECK environment
variable), we should instead make this a non-default behaviour and
those who want it should explicitly opt-in to trigger it, perhaps
like:

        open_editor() {
                if ! test -t 1 && test -n "$GIT_JUMP_AUTO_CAT"
                then
                        cat "$1"
                else
                        editor=`git var GIT_EDITOR`
                        eval "$editor -q \$1"
                fi
        }

so that existing users won't get affected by this change at all,
while allowing you and other vim users to set and export the
environment variable just once.  

Unilaterally breaking, and ignoring when you are told you are
breaking, possible existing users, without giving them any escape
hatch, is simply irresponsible, and not something done in this
project.  But I am sensing that you are not listening to and
thinking about what you hear before you respond, so I will stop
spending time on this topic for now, and wait until others chime in.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

> Imagine a user wanted to spawn a graphical editor via git-jump from within
> a GUI script (probably launched from a window manager menu and has no
> gqipterminal).  With git-jump with this patch, such a use will be broken, no?

Aha OK that's an example that I didn't grok from your earlier mails,
I had only been thinking in the context of editors. The reasoning behind
the "escape hatch" you describe makes sense.

> Unilaterally breaking, and ignoring when you are told you are
> breaking, possible existing users, without giving them any escape
> hatch, is simply irresponsible, and not something done in this
> project.  But I am sensing that you are not listening to and
> thinking about what you hear before you respond, so I will stop
> spending time on this topic for now, and wait until others chime in.

I apologise for not understanding, it was not my intention to be
willfully ignorant.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Because this is a new feature, instead of breaking existing users
> and forcing them to do something different they did not have to
> (namely, set and export the GIT_JUMP_IGNORE_STDOUT_CHECK environment
> variable), we should instead make this a non-default behaviour and
> those who want it should explicitly opt-in to trigger it, perhaps
> like:
>
>         open_editor() {
>                 if ! test -t 1 && test -n "$GIT_JUMP_AUTO_CAT"
>                 then
>                         cat "$1"
>                 else
>                         editor=`git var GIT_EDITOR`
>                         eval "$editor -q \$1"
>                 fi
>         }
>
> so that existing users won't get affected by this change at all,
> while allowing you and other vim users to set and export the
> environment variable just once.  
>
> Unilaterally breaking, and ignoring when you are told you are
> breaking, possible existing users, without giving them any escape
> hatch, is simply irresponsible, and not something done in this
> project.  But I am sensing that you are not listening to and
> thinking about what you hear before you respond, so I will stop
> spending time on this topic for now, and wait until others chime in.

Well, I lied ;-) 

I somehow doubt that users of vim types "!git jump diff" (or
whichever submode they want) from within vim's command prompt;
wouldn't they typically wrap the invocation in a vim macro?

If my suspicion is correct, with an opt-in feature like the above
(which is designed not to hurt existing users), the vim users can
change their macro definition to not just invoke "git jump
<whatever>", but invoke "GIT_JUMP_AUTO_CAT=yes git jump <whatever>",
i.e. tell "git jump" that you are opting into the "cat the file,
instead of launching GIT_EDITOR".  So with just a one-time setting,
vim (and other similar editor) users would benefit without hurting
others.

For that matter, instead of introducing GIT_JUMP_AUTO_CAT, the same
mechanism can be used to run "GIT_EDITOR=cat git jump <whatever>",
i.e. tell "git jump" that it is expected to run "cat" as its
editor, from such a vim macro ;-)

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[George Brown]

> I somehow doubt that users of vim types "!git jump diff" (or
> whichever submode they want) from within vim's command prompt;
> wouldn't they typically wrap the invocation in a vim macro?

Correct, in Vim parlance we'd create a command for this.

> If my suspicion is correct, with an opt-in feature like the above
> (which is designed not to hurt existing users), the vim users can
> change their macro definition to not just invoke "git jump
> <whatever>", but invoke "GIT_JUMP_AUTO_CAT=yes git jump <whatever>",
> i.e. tell "git jump" that you are opting into the "cat the file,
> instead of launching GIT_EDITOR".  So with just a one-time setting,
> vim (and other similar editor) users would benefit without hurting
> others.
>
> For that matter, instead of introducing GIT_JUMP_AUTO_CAT, the same
> mechanism can be used to run "GIT_EDITOR=cat git jump <whatever>",
> i.e. tell "git jump" that it is expected to run "cat" as its
> editor, from such a vim macro ;-)

Yes. Another version that someone else implemented used similar method
by unsetting "$GIT_EDITOR" when invoked from Vim and modified "git jump"
to use cat when "$GIT_EDITOR" was empty.

https://gist.github.com/romainl/a3ddb1d08764b93183260f8cdf0f524f/e1f548f6d96cd6ee97c3daadb4a1546fab7814ad

I can request the author submit that as a patch if it is of interest.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

George Brown <321.george@gmail.com> writes:

> Yes. Another version that someone else implemented used similar method
> by unsetting "$GIT_EDITOR" when invoked from Vim and modified "git jump"
> to use cat when "$GIT_EDITOR" was empty.

That's curious, because with the same approach, that someone else
could have gone one step further to set GIT_EDITOR to 'cat' when vim
invokes via a macro and then there is no need to modify git-jump at
all.  "Curious" because I would intuitively think that unsetting
GIT_EDITOR and setting it to a specific value, 'cat', would require
about the same amount of effort.

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Jeff King]

On Sun, May 10, 2020 at 07:34:09PM +0100, George Brown wrote:

> Arguably the most interoperable way for "git jump" to work would be to
> output the formatted lines and do nothing else, leaving it to users to
> choose how to operate upon the output/invoke editors. Of course such
> a change would break the workflow of anyone who uses "git jump" today
> and isn't a valid option.

Looks like I missed an exciting thread, but let me add a few thoughts as
the author of git-jump:

 - I had no inkling that people would run it from within vim; I assumed
   they'd use more integrated tools like fugitive.vim. ;) I don't think
   it's wrong to do so, though.

 - The paragraph quoted above is the heart of the matter. The tool does
   two things: generate the list and open the editor. My thinking in
   combining them was that "generate the list" was pretty simple and
   just used existing tools anyway, but we have grown a _bit_ of a
   complexity over the years that might make it worth using.

   IMHO the right solution here is a command-line option to say "don't
   start an editor, just send output". Setting GIT_EDITOR=cat
   accomplishes the same thing, but it's much less obvious/discoverable.

 - I'm pretty sure git-jump does _not_ work with emacs or emacsclient.
   Somebody corresponded with me off-list about patches many years ago,
   but I can't remember what the hold-up was. If anybody is interested
   in pushing that forward, I'd be happy to dig it out my archive.

   However, it should work with gvim, and any isatty() check would
   potentially cause issues there. So I'd much prefer the caller say
   explicitly that they're not expecting the editor to start.

So I'm OK to leave the status quo and let people use the GIT_EDITOR
solution in this instance. But I'd also be happy to take a patch for
"--no-editor" or similar if somebody wants to work it up.

-Peff

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>  - I'm pretty sure git-jump does _not_ work with emacs or emacsclient.

;-)  Its output can be used in M-x find-grep, though, actually.

>    However, it should work with gvim, and any isatty() check would
>    potentially cause issues there. So I'd much prefer the caller say
>    explicitly that they're not expecting the editor to start.

Yes, that is exactly what I was worried about.

> So I'm OK to leave the status quo and let people use the GIT_EDITOR
> solution in this instance. But I'd also be happy to take a patch for
> "--no-editor" or similar if somebody wants to work it up.

I actually would support --no-editor.  One thing nobody noticed so
far is that "git-jump" is only compatible with editors that support
the "-q" option from the command line, and "cat" is not among them.


Another thing I was thinking about was a change like the attached.
Plugging it thru "git var" to allow "git var GIT_JUMP_EDITOR" may
allow vim users to set it to 'cat' while setting GIT_EDITOR to vim.

It still needs a fix to get rid of the uncondtional "-q" option from
open_editor() function, though.


 cache.h  |  2 +-
 editor.c | 33 ++++++++++++++++++++++-----------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index 0f0485ecfe..b3f7a6d6c4 100644
--- a/cache.h
+++ b/cache.h
@@ -1649,7 +1649,7 @@ const char *fmt_ident(const char *name, const char *email,
 const char *fmt_name(enum want_ident);
 const char *ident_default_name(void);
 const char *ident_default_email(void);
-const char *git_editor(void);
+const char *git_editor(const char *);
 const char *git_sequence_editor(void);
 const char *git_pager(int stdout_is_tty);
 int is_terminal_dumb(void);
diff --git a/editor.c b/editor.c
index 91989ee8a1..3d04824e9e 100644
--- a/editor.c
+++ b/editor.c
@@ -14,11 +14,29 @@ int is_terminal_dumb(void)
 	return !terminal || !strcmp(terminal, "dumb");
 }
 
-const char *git_editor(void)
+const char *git_editor(const char *program)
 {
-	const char *editor = getenv("GIT_EDITOR");
+	const char *editor = NULL;
 	int terminal_is_dumb = is_terminal_dumb();
 
+	if (program) {
+		char *varname;
+
+		varname = xstrfmt("GIT_%s_EDITOR", program);
+		editor = getenv(varname);
+		free(varname);
+
+		if (!editor) {
+			struct strbuf progname = STRBUF_INIT;
+			strbuf_addstr(&progname, program);
+			strbuf_tolower(&progname);
+			varname = xstrfmt("%s.editor", progname.buf);
+			git_config_get_string_const(varname, &editor);
+			free(varname);
+			strbuf_release(&progname);
+		}
+	}
+
 	if (!editor && editor_program)
 		editor = editor_program;
 	if (!editor && !terminal_is_dumb)
@@ -37,14 +55,7 @@ const char *git_editor(void)
 
 const char *git_sequence_editor(void)
 {
-	const char *editor = getenv("GIT_SEQUENCE_EDITOR");
-
-	if (!editor)
-		git_config_get_string_const("sequence.editor", &editor);
-	if (!editor)
-		editor = git_editor();
-
-	return editor;
+	return git_editor("SEQUENCE");
 }
 
 static int launch_specified_editor(const char *editor, const char *path,
@@ -118,7 +129,7 @@ static int launch_specified_editor(const char *editor, const char *path,
 
 int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
 {
-	return launch_specified_editor(git_editor(), path, buffer, env);
+	return launch_specified_editor(git_editor(NULL), path, buffer, env);
 }
 
 int launch_sequence_editor(const char *path, struct strbuf *buffer,

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Jeff King]

On Mon, May 11, 2020 at 08:36:05AM -0700, Junio C Hamano wrote:

> > So I'm OK to leave the status quo and let people use the GIT_EDITOR
> > solution in this instance. But I'd also be happy to take a patch for
> > "--no-editor" or similar if somebody wants to work it up.
> 
> I actually would support --no-editor.  One thing nobody noticed so
> far is that "git-jump" is only compatible with editors that support
> the "-q" option from the command line, and "cat" is not among them.

Oh, good point. GIT_EDITOR='cat -- 2>/dev/null' works, but is rather
obscure. :)

> Another thing I was thinking about was a change like the attached.
> Plugging it thru "git var" to allow "git var GIT_JUMP_EDITOR" may
> allow vim users to set it to 'cat' while setting GIT_EDITOR to vim.

I wouldn't use it myself, but I don't have any objection to adding
support.

>  cache.h  |  2 +-
>  editor.c | 33 ++++++++++++++++++++++-----------

You'd presumably need to make git-var understand that GIT_JUMP_EDITOR
should fall back to GIT_EDITOR.

-Peff

Re* [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, May 11, 2020 at 08:36:05AM -0700, Junio C Hamano wrote:
>
>> > So I'm OK to leave the status quo and let people use the GIT_EDITOR
>> > solution in this instance. But I'd also be happy to take a patch for
>> > "--no-editor" or similar if somebody wants to work it up.
>> 
>> I actually would support --no-editor.  One thing nobody noticed so
>> far is that "git-jump" is only compatible with editors that support
>> the "-q" option from the command line, and "cat" is not among them.
>
> Oh, good point. GIT_EDITOR='cat -- 2>/dev/null' works, but is rather
> obscure. :)

Lest we all forget...

-- >8 --
Subject: git-jump: just show the list with the "--no-editor" option

The "git jump" script (in contrib/) creates a list of interesting
places to be visited in an editor, and then open the editor to visit
the place.  Some editors, however, can read the list directly and
use it to visit these places (e.g. vim's quickfix list, or emacs's
find-grep/compilation buffer) and do not want "git jump" to invoke
a separate editor.

Users can _almost_ do this already by setting GIT_EDITOR to "cat",
except that "git jump" assumes that the editor it spawns support a
"-q" option from the command line, and unfortunately "cat" is not
among such editors.

Teach it the "--no-editor" option, which tells the command to show
the list it generated to its standard output.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/git-jump/git-jump | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
index 931b0fe3a9..26a7159053 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 [--no-editor] <mode> [<args>]
 
 Jump to interesting elements in an editor.
 The <mode> parameter is one of:
@@ -64,6 +64,18 @@ mode_ws() {
 	git diff --check "$@"
 }
 
+edit=yes
+
+while	case "$#,$1" in
+	0,*) break ;;
+	*,--no-editor) edit=no ;;
+	*,--*) usage >&2; exit 1 ;;
+	*) break ;;
+	esac
+do
+	shift
+done
+
 if test $# -lt 1; then
 	usage >&2
 	exit 1
@@ -75,4 +87,9 @@ 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"
+
+case "$edit" in
+yes)	open_editor "$tmp" ;;
+no)	cat "$tmp" ;;
+esac
+

Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal

[Jeff King]

On Mon, May 11, 2020 at 09:46:34AM -0700, Junio C Hamano wrote:

> Lest we all forget...
> 
> -- >8 --
> Subject: git-jump: just show the list with the "--no-editor" option

Thanks for tying this up. It seems to work as advertised. A few nits:

> +edit=yes
> +
> +while	case "$#,$1" in

Tab between "while" and "case"?

> +	0,*) break ;;
> +	*,--no-editor) edit=no ;;
> +	*,--*) usage >&2; exit 1 ;;
> +	*) break ;;
> +	esac
> +do
> +	shift
> +done

I found the use of "case" in the loop conditional a little unusual. I'd
have probably written:

  while test $# -gt 0
  do
	case "$1" in
	--no-editor) edit=no ;;
	--*) usage >&2; exit 1 ;;
	*) break ;;
	esac
	shift
  done

> @@ -75,4 +87,9 @@ 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"
> +
> +case "$edit" in
> +yes)	open_editor "$tmp" ;;
> +no)	cat "$tmp" ;;
> +esac
> +

"diff --check" complains about the empty line.

It probably doesn't matter much, but we could skip the tempfile entirely
in no-editor mode. I.e.:

  if test "$edit" = "no"
  then
    "mode_$mode" "$@"
  fi

  # otherwise set up trap, mktemp, etc

-Peff

Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, May 11, 2020 at 09:46:34AM -0700, Junio C Hamano wrote:
>
>> Lest we all forget...
>> 
>> -- >8 --
>> Subject: git-jump: just show the list with the "--no-editor" option
>
> Thanks for tying this up. It seems to work as advertised. A few nits:
>
>> +edit=yes
>> +
>> +while	case "$#,$1" in
>
> Tab between "while" and "case"?

Yup.  Just to align case and its arms.

>> +	0,*) break ;;
>> +	*,--no-editor) edit=no ;;
>> +	*,--*) usage >&2; exit 1 ;;
>> +	*) break ;;
>> +	esac
>> +do
>> +	shift
>> +done
>
> I found the use of "case" in the loop conditional a little unusual.

It's pretty-much personal preference, I think.  I could replace
s/break/false/ if you find it easier to understand.

> It probably doesn't matter much, but we could skip the tempfile entirely
> in no-editor mode. I.e.:
>
>   if test "$edit" = "no"
>   then
>     "mode_$mode" "$@"
>   fi
>
>   # otherwise set up trap, mktemp, etc

Makes a lot of sense to me.

Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal

[Jeff King]

On Tue, May 12, 2020 at 02:30:34PM -0700, Junio C Hamano wrote:

> >> +edit=yes
> >> +
> >> +while	case "$#,$1" in
> >
> > Tab between "while" and "case"?
> 
> Yup.  Just to align case and its arms.

I guess that makes sense, though I'd probably have used spaces to do so.
Having a tab in the middle of the line is unusual.

> >> +	0,*) break ;;
> >> +	*,--no-editor) edit=no ;;
> >> +	*,--*) usage >&2; exit 1 ;;
> >> +	*) break ;;
> >> +	esac
> >> +do
> >> +	shift
> >> +done
> >
> > I found the use of "case" in the loop conditional a little unusual.
> 
> It's pretty-much personal preference, I think.  I could replace
> s/break/false/ if you find it easier to understand.

I think the part that most threw me off is looking at the arg-count in
each case arm. It's "*" in most, which really means "do not bother to
look at it" (which I think is why I found a loop condition on "$# -gt 0"
to be more natural).

I can live with it either way.

-Peff

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Benjamin]

Junio writes:

> I somehow doubt that users of vim types "!git jump diff" (or
> whichever submode they want) from within vim's command prompt;
> wouldn't they typically wrap the invocation in a vim macro?

Vim-user here. I run "git jump (options)" from a shell quite a bit, but when I'm
in vim I tend to use ":Ggrep" and similar commands from the fugitive plugin [1].
I don't *think* I'm alone in this.

I'm not really arguing either side, just pointing out that other workflows exist
(and indeed, I'm unlikely to run ":!git jump diff").

[1]: https://github.com/tpope/vim-fugitive

D. Ben Knoble

Re: [PATCH] contrib/git-jump: cat output when not a terminal

[Junio C Hamano]

Benjamin <ben.knoble@gmail.com> writes:

> Junio writes:
>
>> I somehow doubt that users of vim types "!git jump diff" (or
>> whichever submode they want) from within vim's command prompt;
>> wouldn't they typically wrap the invocation in a vim macro?
>
> Vim-user here. I run "git jump (options)" from a shell quite a bit, but when I'm
> in vim I tend to use ":Ggrep" and similar commands from the fugitive plugin [1].
> I don't *think* I'm alone in this.

I can believe there are tons of users of the plugin.  My point still
stands---such a plugin can export GIT_EDITOR=cat when running "git
jump" and there is no need to change "git jump".