{"thread":{"id":"53437","subject":"[PATCH] contrib/git-jump: cat output when not a terminal","startedAt":"2020-05-09T19:15:54Z","lastAt":"2020-05-13T04:52:40Z","messageCount":21,"participants":["George Brown","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"397467","messageId":"CAFKec1W0-OOQYypP-3VC=dJnuNDrykdQ2xibc=u4D=Zo6if-+Q@mail.gmail.com","threadId":"53437","inReplyTo":null,"subject":"[PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-09T19:15:40Z","receivedAt":"2020-05-09T19:15:54Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"contrib/git-jump: cat output when not a terminal\n\nThe current usage to populate Vim's quickfix list cannot be used from\nwithin the editor as it invokes another instance.\n\nCheck if stdout is to a terminal or not. If not simply cat the output.\n\nSigned-off-by: George Brown <321.george@gmail.com>\n---\n contrib/git-jump/git-jump | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 931b0fe3a9..253341c64e 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -19,8 +19,12 @@ EOF\n }\n\n open_editor() {\n-    editor=`git var GIT_EDITOR`\n-    eval \"$editor -q \\$1\"\n+    if test -t 1; then\n+        editor=`git var GIT_EDITOR`\n+        eval \"$editor -q \\$1\"\n+    else\n+        cat \"$1\"\n+    fi\n }\n\n mode_diff() {\n"},{"id":"397472","messageId":"xmqqd07cvl9b.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"CAFKec1W0-OOQYypP-3VC=dJnuNDrykdQ2xibc=u4D=Zo6if-+Q@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-09T21:41:52Z","receivedAt":"2020-05-09T21:42:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Brown <321.george@gmail.com> writes:\n\n> contrib/git-jump: cat output when not a terminal\n>\n> The current usage to populate Vim's quickfix list cannot be used from\n> within the editor as it invokes another instance.\n>\n> Check if stdout is to a terminal or not. If not simply cat the output.\n\nThe are editors that open their own window, or tell an already open\neditor instance to edit the buffer (think: emacsclient) and in order\nto be functional, they do not have to be launched from inside a\nworking terminal window (whose input comes from the keyboard---the\ntest \"-t 1\" is an approximate check for that).\n\nWouldn't this change hurt the users of such an editor?\n\nWould it work to set GIT_EDITOR to \"cat\" while performing the\n\"populating quickfix list\" (whatever that is) operation?\n\n> Signed-off-by: George Brown <321.george@gmail.com>\n> ---\n>  contrib/git-jump/git-jump | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> index 931b0fe3a9..253341c64e 100755\n> --- a/contrib/git-jump/git-jump\n> +++ b/contrib/git-jump/git-jump\n> @@ -19,8 +19,12 @@ EOF\n>  }\n>\n>  open_editor() {\n> -    editor=`git var GIT_EDITOR`\n> -    eval \"$editor -q \\$1\"\n> +    if test -t 1; then\n> +        editor=`git var GIT_EDITOR`\n> +        eval \"$editor -q \\$1\"\n> +    else\n> +        cat \"$1\"\n> +    fi\n>  }\n>\n>  mode_diff() {\n"},{"id":"397473","messageId":"CAFKec1Wj_uK-moVfin3XrTEmmBaAzaJKsh1f8q-3+RBs2-3Jdg@mail.gmail.com","threadId":"53437","inReplyTo":"xmqqd07cvl9b.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-09T22:04:15Z","receivedAt":"2020-05-09T22:04:42Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"> The are editors that open their own window, or tell an already open\n> editor instance to edit the buffer (think: emacsclient) and in order\n> to be functional, they do not have to be launched from inside a\n> working terminal window (whose input comes from the keyboard---the\n> test \"-t 1\" is an approximate check for that).\n>\n> Wouldn't this change hurt the users of such an editor?\n\nI'm unsure of emacsclient but I would have thought that editors in\ngeneral would be more amenable to receiving lines than relying on\nadditional spawned instances to behave as clients.\n\n> Would it work to set GIT_EDITOR to \"cat\" while performing the\n> \"populating quickfix list\" (whatever that is) operation?\n\nIt would, though I considered this to be less elegant.\n\nJust for background the quickfix list in Vim is simply a list of\nlocations that you can browse and jump to.\n\nOn Sat, 9 May 2020 at 22:41, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> George Brown <321.george@gmail.com> writes:\n>\n> > contrib/git-jump: cat output when not a terminal\n> >\n> > The current usage to populate Vim's quickfix list cannot be used from\n> > within the editor as it invokes another instance.\n> >\n> > Check if stdout is to a terminal or not. If not simply cat the output.\n>\n> The are editors that open their own window, or tell an already open\n> editor instance to edit the buffer (think: emacsclient) and in order\n> to be functional, they do not have to be launched from inside a\n> working terminal window (whose input comes from the keyboard---the\n> test \"-t 1\" is an approximate check for that).\n>\n> Wouldn't this change hurt the users of such an editor?\n>\n> Would it work to set GIT_EDITOR to \"cat\" while performing the\n> \"populating quickfix list\" (whatever that is) operation?\n>\n> > Signed-off-by: George Brown <321.george@gmail.com>\n> > ---\n> >  contrib/git-jump/git-jump | 8 ++++++--\n> >  1 file changed, 6 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> > index 931b0fe3a9..253341c64e 100755\n> > --- a/contrib/git-jump/git-jump\n> > +++ b/contrib/git-jump/git-jump\n> > @@ -19,8 +19,12 @@ EOF\n> >  }\n> >\n> >  open_editor() {\n> > -    editor=`git var GIT_EDITOR`\n> > -    eval \"$editor -q \\$1\"\n> > +    if test -t 1; then\n> > +        editor=`git var GIT_EDITOR`\n> > +        eval \"$editor -q \\$1\"\n> > +    else\n> > +        cat \"$1\"\n> > +    fi\n> >  }\n> >\n> >  mode_diff() {\n"},{"id":"397475","messageId":"xmqq8si0vfp3.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"CAFKec1Wj_uK-moVfin3XrTEmmBaAzaJKsh1f8q-3+RBs2-3Jdg@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-09T23:42:00Z","receivedAt":"2020-05-09T23:42:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Brown <321.george@gmail.com> writes:\n\n>> The are editors that open their own window, or tell an already open\n>> editor instance to edit the buffer (think: emacsclient) and in order\n>> to be functional, they do not have to be launched from inside a\n>> working terminal window (whose input comes from the keyboard---the\n>> test \"-t 1\" is an approximate check for that).\n>>\n>> Wouldn't this change hurt the users of such an editor?\n>\n> I'm unsure of emacsclient but I would have thought that editors in\n> general would be more amenable to receiving lines than relying on\n> additional spawned instances to behave as clients.\n\nI am not sure what you mean by \"receiving lines\".  Your patch makes\nthe \"git jump\" command completely ignore configured GIT_EDITOR and\nrefuse to invoke *any* editor.\n\n\n\n\n"},{"id":"397486","messageId":"CAFKec1UGKbaV7wC78i8+uSEizjGkj2bDSfOeucvJORhORvc5KA@mail.gmail.com","threadId":"53437","inReplyTo":"xmqq8si0vfp3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-10T09:03:59Z","receivedAt":"2020-05-10T09:04:13Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"> I am not sure what you mean by \"receiving lines\".  Your patch makes\n> the \"git jump\" command completely ignore configured GIT_EDITOR and\n> refuse to invoke *any* editor.\n\nRunning \"git jump\" from a shell prompt works as before with this patch.\n\nBut now within Vim it can also be used to populate the quickfix list\nwith something like \":cexpr system('git jump')\". Because instead of\ninvoking another instance of (which does not work well for Vim) it\njust receives the lines as formatted by \"git jump\".\n\n> > Would it work to set GIT_EDITOR to \"cat\" while performing the\n> > \"populating quickfix list\" (whatever that is) operation?\n>\n> It would, though I considered this to be less elegant.\n\nJust to revise this, it wouldn't actually as the editor is always passed\n\"-q\" (intended for Vim's quickfix list). So overriding this to \"cat\"\ncauses as error as \"-q\" is not a valid option for it.\n\nI'm aware the examples I'm giving are specific to Vim but it really\nseems that \"git jump\" was intended with Vim in mind. That said I'd argue\nmost other editors would work better with \"git jump\" if when they\ninvoked it another instance wasn't created. I've tried find an\nequivalent to Vim's quickfix in Emacs this morning but can't quite wrap\nmy head around it.\n"},{"id":"397505","messageId":"xmqqwo5ju47t.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"CAFKec1UGKbaV7wC78i8+uSEizjGkj2bDSfOeucvJORhORvc5KA@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-10T16:47:34Z","receivedAt":"2020-05-10T16:47:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Brown <321.george@gmail.com> writes:\n\n> Running \"git jump\" from a shell prompt works as before with this patch.\n\nI am not worried at all about that use case.  It is clear that you\nhad the \"shell prompt\" in mind from \"test -t 1\".\n\nGiven what \"git jump\" does, wouldn't it be natural to expect that\nGUI programs that have no \"shell prompt\" interaction with the end\nuser want to use \"git jump\"?  It may even be driving a graphical\nversion of vim---or is it impossible for such a GUI program to\nexist?\n\nAs long as such a regression for existing users use is impossible, I\nthink the patch is probably OK, but doing a hardcoded \"cat\" smells\nlike a very bad hack, compared to a solution on the program's side\nthat *wants* to read the prepared file to arrange that to happen\n(e.g. via setting the GIT_EDITOR environment to \"cat\" within that\nprogram).\n\nIn any case, I am not a \"git jump\" user, so...\n\n"},{"id":"397507","messageId":"CAFKec1Wy1iT8Z=gNDBn++XLxzGWr0UUiu3AKMU-qaR+jj2yoKQ@mail.gmail.com","threadId":"53437","inReplyTo":"xmqqwo5ju47t.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-10T17:33:12Z","receivedAt":"2020-05-10T17:33:26Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"> As long as such a regression for existing users use is impossible, I\n> think the patch is probably OK, but doing a hardcoded \"cat\" smells\n> like a very bad hack, compared to a solution on the program's side\n> that *wants* to read the prepared file to arrange that to happen\n> (e.g. via setting the GIT_EDITOR environment to \"cat\" within that\n> program).\n\nI think this change only enhances \"git jump\".\n\n> In any case, I am not a \"git jump\" user, so...\n\nI looked into this as other Vim users were talking about it and wanted\nthis behavior.\n\nhttps://www.reddit.com/r/vim/comments/gf2he2/what_is_the_simplest_way_to_get_all_git_changes/fprlxtk/?context=3\n"},{"id":"397508","messageId":"xmqqo8qvu0ao.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"CAFKec1Wy1iT8Z=gNDBn++XLxzGWr0UUiu3AKMU-qaR+jj2yoKQ@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-10T18:12:15Z","receivedAt":"2020-05-10T18:12:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Brown <321.george@gmail.com> writes:\n\n>> As long as such a regression for existing users use is impossible, I\n>> think the patch is probably OK, but doing a hardcoded \"cat\" smells\n>> like a very bad hack, compared to a solution on the program's side\n>> that *wants* to read the prepared file to arrange that to happen\n>> (e.g. via setting the GIT_EDITOR environment to \"cat\" within that\n>> program).\n>\n> I think this change only enhances \"git jump\".\n>\n>> In any case, I am not a \"git jump\" user, so...\n>\n> I looked into this as other Vim users were talking about it and wanted\n> this behavior.\n\nThat's already irrelevant in the course of this discussion, no?\n\nWe have already established that you wrote in good faith to improve\nthe use experience by vim users, and nobody in this exchange doubts\nthat this change would help vim users.  \n\nI am worried about the change hurting non-vim users, but no amount\nof \"me too\" from vim users who care only about their vim experience\nwould allay that.\n"},{"id":"397509","messageId":"CAFKec1VGzpxVJV4zak46r_p2gGcw4UanFr7U4U4MSsG7t2A23w@mail.gmail.com","threadId":"53437","inReplyTo":"xmqqo8qvu0ao.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-10T18:34:09Z","receivedAt":"2020-05-10T18:34:24Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"I think with this change all editors can benefit. The format \"git jump\"\nis producing is something easily consumed. I think consumption of output\nfrom tools is far more common in editors than communication between\nmultiple instances.\n\nAs an aside the fact that as is \"git jump\" invokes \"$GIT_EDITOR\" with\nthe \"-q\" option makes an implicit assumption the editor will be Vim or\nsomething very much like it. To be very clear I don't mean to say this\nmeans only Vim should be considered. However it's also making the\nimplicit assumption that passing the \"-q\" option is valid for any\n\"$GIT_EDITOR\" and does not cause an error like that seen when trying to\noverride \"$GIT_EDITOR\" with cat. This change means other editors can\ninvoke \"git jump\" without fear of such a situation, increasing\nusability.\n\nArguably the most interoperable way for \"git jump\" to work would be to\noutput the formatted lines and do nothing else, leaving it to users to\nchoose how to operate upon the output/invoke editors. Of course such\na change would break the workflow of anyone who uses \"git jump\" today\nand isn't a valid option.\n"},{"id":"397511","messageId":"xmqqk11jtxl3.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"CAFKec1VGzpxVJV4zak46r_p2gGcw4UanFr7U4U4MSsG7t2A23w@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-10T19:10:48Z","receivedAt":"2020-05-10T19:10:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Brown <321.george@gmail.com> writes:\n\n> I think with this change all editors can benefit.\n\nI am worried not only about the editors that are launched by\ngit-jump that runs GIT_EDITOR, or the use case where you run\ngit-jump from within your editor.  \"git jump\" that is launched from\na process, whose standard output steam is not connected to a\nterminal, used to spawn the specified editor just fine.  Imagine a\nuser wanted to spawn a graphical editor via git-jump from within a\nGUI script (probably launched from a window manager menu and has no\nterminal).  With git-jump with this patch, such a use will be\nbroken, no?  The GIT_EDITOR the user set is totally ignored.\n\nAt the very least, we should have an escape hatch to help those this\npatch is hurting, perhaps like\n\n        open_editor() {\n                if ! test -t 1 && test -z \"$GIT_JUMP_IGNORE_STDOUT_CHECK\"\n                then\n                        cat \"$1\"\n                else\n                        editor=`git var GIT_EDITOR`\n                        eval \"$editor -q \\$1\"\n                fi\n        }\n\nso that at least they can spawn their chosen editor, not \"cat\", from\nthe tool they have been using.  \n\nBecause this is a new feature, instead of breaking existing users\nand forcing them to do something different they did not have to\n(namely, set and export the GIT_JUMP_IGNORE_STDOUT_CHECK environment\nvariable), we should instead make this a non-default behaviour and\nthose who want it should explicitly opt-in to trigger it, perhaps\nlike:\n\n        open_editor() {\n                if ! test -t 1 && test -n \"$GIT_JUMP_AUTO_CAT\"\n                then\n                        cat \"$1\"\n                else\n                        editor=`git var GIT_EDITOR`\n                        eval \"$editor -q \\$1\"\n                fi\n        }\n\nso that existing users won't get affected by this change at all,\nwhile allowing you and other vim users to set and export the\nenvironment variable just once.  \n\nUnilaterally breaking, and ignoring when you are told you are\nbreaking, possible existing users, without giving them any escape\nhatch, is simply irresponsible, and not something done in this\nproject.  But I am sensing that you are not listening to and\nthinking about what you hear before you respond, so I will stop\nspending time on this topic for now, and wait until others chime in.\n\n"},{"id":"397512","messageId":"CAFKec1Uya31withxzAqwFSVxqgxj_RbU1B6NrMYQ4U8WzLOs6g@mail.gmail.com","threadId":"53437","inReplyTo":"xmqqk11jtxl3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-10T19:25:59Z","receivedAt":"2020-05-10T19:26:38Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"> Imagine a user wanted to spawn a graphical editor via git-jump from within\n> a GUI script (probably launched from a window manager menu and has no\n> gqipterminal).  With git-jump with this patch, such a use will be broken, no?\n\nAha OK that's an example that I didn't grok from your earlier mails,\nI had only been thinking in the context of editors. The reasoning behind\nthe \"escape hatch\" you describe makes sense.\n\n> Unilaterally breaking, and ignoring when you are told you are\n> breaking, possible existing users, without giving them any escape\n> hatch, is simply irresponsible, and not something done in this\n> project.  But I am sensing that you are not listening to and\n> thinking about what you hear before you respond, so I will stop\n> spending time on this topic for now, and wait until others chime in.\n\nI apologise for not understanding, it was not my intention to be\nwillfully ignorant.\n"},{"id":"397513","messageId":"xmqqftc7twaa.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"xmqqk11jtxl3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-10T19:38:53Z","receivedAt":"2020-05-10T19:39:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Because this is a new feature, instead of breaking existing users\n> and forcing them to do something different they did not have to\n> (namely, set and export the GIT_JUMP_IGNORE_STDOUT_CHECK environment\n> variable), we should instead make this a non-default behaviour and\n> those who want it should explicitly opt-in to trigger it, perhaps\n> like:\n>\n>         open_editor() {\n>                 if ! test -t 1 && test -n \"$GIT_JUMP_AUTO_CAT\"\n>                 then\n>                         cat \"$1\"\n>                 else\n>                         editor=`git var GIT_EDITOR`\n>                         eval \"$editor -q \\$1\"\n>                 fi\n>         }\n>\n> so that existing users won't get affected by this change at all,\n> while allowing you and other vim users to set and export the\n> environment variable just once.  \n>\n> Unilaterally breaking, and ignoring when you are told you are\n> breaking, possible existing users, without giving them any escape\n> hatch, is simply irresponsible, and not something done in this\n> project.  But I am sensing that you are not listening to and\n> thinking about what you hear before you respond, so I will stop\n> spending time on this topic for now, and wait until others chime in.\n\nWell, I lied ;-) \n\nI somehow doubt that users of vim types \"!git jump diff\" (or\nwhichever submode they want) from within vim's command prompt;\nwouldn't they typically wrap the invocation in a vim macro?\n\nIf my suspicion is correct, with an opt-in feature like the above\n(which is designed not to hurt existing users), the vim users can\nchange their macro definition to not just invoke \"git jump\n<whatever>\", but invoke \"GIT_JUMP_AUTO_CAT=yes git jump <whatever>\",\ni.e. tell \"git jump\" that you are opting into the \"cat the file,\ninstead of launching GIT_EDITOR\".  So with just a one-time setting,\nvim (and other similar editor) users would benefit without hurting\nothers.\n\nFor that matter, instead of introducing GIT_JUMP_AUTO_CAT, the same\nmechanism can be used to run \"GIT_EDITOR=cat git jump <whatever>\",\ni.e. tell \"git jump\" that it is expected to run \"cat\" as its\neditor, from such a vim macro ;-)\n"},{"id":"397515","messageId":"CAFKec1VGMwn3_4AEuY8Vrs60UQbX-fvqCkzKd8VARAUScbA=rA@mail.gmail.com","threadId":"53437","inReplyTo":"xmqqftc7twaa.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"George Brown","fromEmail":"321.george@gmail.com","sentAt":"2020-05-10T20:20:00Z","receivedAt":"2020-05-10T20:20:14Z","isPatch":true,"sender":{"key":"321.george@gmail.com","avatar":null},"body":"> I somehow doubt that users of vim types \"!git jump diff\" (or\n> whichever submode they want) from within vim's command prompt;\n> wouldn't they typically wrap the invocation in a vim macro?\n\nCorrect, in Vim parlance we'd create a command for this.\n\n> If my suspicion is correct, with an opt-in feature like the above\n> (which is designed not to hurt existing users), the vim users can\n> change their macro definition to not just invoke \"git jump\n> <whatever>\", but invoke \"GIT_JUMP_AUTO_CAT=yes git jump <whatever>\",\n> i.e. tell \"git jump\" that you are opting into the \"cat the file,\n> instead of launching GIT_EDITOR\".  So with just a one-time setting,\n> vim (and other similar editor) users would benefit without hurting\n> others.\n>\n> For that matter, instead of introducing GIT_JUMP_AUTO_CAT, the same\n> mechanism can be used to run \"GIT_EDITOR=cat git jump <whatever>\",\n> i.e. tell \"git jump\" that it is expected to run \"cat\" as its\n> editor, from such a vim macro ;-)\n\nYes. Another version that someone else implemented used similar method\nby unsetting \"$GIT_EDITOR\" when invoked from Vim and modified \"git jump\"\nto use cat when \"$GIT_EDITOR\" was empty.\n\nhttps://gist.github.com/romainl/a3ddb1d08764b93183260f8cdf0f524f/e1f548f6d96cd6ee97c3daadb4a1546fab7814ad\n\nI can request the author submit that as a patch if it is of interest.\n"},{"id":"397547","messageId":"xmqq5zd2tufn.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"CAFKec1VGMwn3_4AEuY8Vrs60UQbX-fvqCkzKd8VARAUScbA=rA@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-11T14:31:08Z","receivedAt":"2020-05-11T14:31:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Brown <321.george@gmail.com> writes:\n\n> Yes. Another version that someone else implemented used similar method\n> by unsetting \"$GIT_EDITOR\" when invoked from Vim and modified \"git jump\"\n> to use cat when \"$GIT_EDITOR\" was empty.\n\nThat's curious, because with the same approach, that someone else\ncould have gone one step further to set GIT_EDITOR to 'cat' when vim\ninvokes via a macro and then there is no need to modify git-jump at\nall.  \"Curious\" because I would intuitively think that unsetting\nGIT_EDITOR and setting it to a specific value, 'cat', would require\nabout the same amount of effort.\n"},{"id":"397548","messageId":"20200511143157.GA1415@coredump.intra.peff.net","threadId":"53437","inReplyTo":"CAFKec1VGzpxVJV4zak46r_p2gGcw4UanFr7U4U4MSsG7t2A23w@mail.gmail.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-11T14:31:57Z","receivedAt":"2020-05-11T14:31:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 10, 2020 at 07:34:09PM +0100, George Brown wrote:\n\n> Arguably the most interoperable way for \"git jump\" to work would be to\n> output the formatted lines and do nothing else, leaving it to users to\n> choose how to operate upon the output/invoke editors. Of course such\n> a change would break the workflow of anyone who uses \"git jump\" today\n> and isn't a valid option.\n\nLooks like I missed an exciting thread, but let me add a few thoughts as\nthe author of git-jump:\n\n - I had no inkling that people would run it from within vim; I assumed\n   they'd use more integrated tools like fugitive.vim. ;) I don't think\n   it's wrong to do so, though.\n\n - The paragraph quoted above is the heart of the matter. The tool does\n   two things: generate the list and open the editor. My thinking in\n   combining them was that \"generate the list\" was pretty simple and\n   just used existing tools anyway, but we have grown a _bit_ of a\n   complexity over the years that might make it worth using.\n\n   IMHO the right solution here is a command-line option to say \"don't\n   start an editor, just send output\". Setting GIT_EDITOR=cat\n   accomplishes the same thing, but it's much less obvious/discoverable.\n\n - I'm pretty sure git-jump does _not_ work with emacs or emacsclient.\n   Somebody corresponded with me off-list about patches many years ago,\n   but I can't remember what the hold-up was. If anybody is interested\n   in pushing that forward, I'd be happy to dig it out my archive.\n\n   However, it should work with gvim, and any isatty() check would\n   potentially cause issues there. So I'd much prefer the caller say\n   explicitly that they're not expecting the editor to start.\n\nSo I'm OK to leave the status quo and let people use the GIT_EDITOR\nsolution in this instance. But I'd also be happy to take a patch for\n\"--no-editor\" or similar if somebody wants to work it up.\n\n-Peff\n"},{"id":"397554","messageId":"xmqqr1vqscuy.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"20200511143157.GA1415@coredump.intra.peff.net","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-11T15:36:05Z","receivedAt":"2020-05-11T15:36:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>  - I'm pretty sure git-jump does _not_ work with emacs or emacsclient.\n\n;-)  Its output can be used in M-x find-grep, though, actually.\n\n>    However, it should work with gvim, and any isatty() check would\n>    potentially cause issues there. So I'd much prefer the caller say\n>    explicitly that they're not expecting the editor to start.\n\nYes, that is exactly what I was worried about.\n\n> So I'm OK to leave the status quo and let people use the GIT_EDITOR\n> solution in this instance. But I'd also be happy to take a patch for\n> \"--no-editor\" or similar if somebody wants to work it up.\n\nI actually would support --no-editor.  One thing nobody noticed so\nfar is that \"git-jump\" is only compatible with editors that support\nthe \"-q\" option from the command line, and \"cat\" is not among them.\n\n\nAnother thing I was thinking about was a change like the attached.\nPlugging it thru \"git var\" to allow \"git var GIT_JUMP_EDITOR\" may\nallow vim users to set it to 'cat' while setting GIT_EDITOR to vim.\n\nIt still needs a fix to get rid of the uncondtional \"-q\" option from\nopen_editor() function, though.\n\n\n cache.h  |  2 +-\n editor.c | 33 ++++++++++++++++++++++-----------\n 2 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 0f0485ecfe..b3f7a6d6c4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1649,7 +1649,7 @@ const char *fmt_ident(const char *name, const char *email,\n const char *fmt_name(enum want_ident);\n const char *ident_default_name(void);\n const char *ident_default_email(void);\n-const char *git_editor(void);\n+const char *git_editor(const char *);\n const char *git_sequence_editor(void);\n const char *git_pager(int stdout_is_tty);\n int is_terminal_dumb(void);\ndiff --git a/editor.c b/editor.c\nindex 91989ee8a1..3d04824e9e 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -14,11 +14,29 @@ int is_terminal_dumb(void)\n \treturn !terminal || !strcmp(terminal, \"dumb\");\n }\n \n-const char *git_editor(void)\n+const char *git_editor(const char *program)\n {\n-\tconst char *editor = getenv(\"GIT_EDITOR\");\n+\tconst char *editor = NULL;\n \tint terminal_is_dumb = is_terminal_dumb();\n \n+\tif (program) {\n+\t\tchar *varname;\n+\n+\t\tvarname = xstrfmt(\"GIT_%s_EDITOR\", program);\n+\t\teditor = getenv(varname);\n+\t\tfree(varname);\n+\n+\t\tif (!editor) {\n+\t\t\tstruct strbuf progname = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&progname, program);\n+\t\t\tstrbuf_tolower(&progname);\n+\t\t\tvarname = xstrfmt(\"%s.editor\", progname.buf);\n+\t\t\tgit_config_get_string_const(varname, &editor);\n+\t\t\tfree(varname);\n+\t\t\tstrbuf_release(&progname);\n+\t\t}\n+\t}\n+\n \tif (!editor && editor_program)\n \t\teditor = editor_program;\n \tif (!editor && !terminal_is_dumb)\n@@ -37,14 +55,7 @@ const char *git_editor(void)\n \n const char *git_sequence_editor(void)\n {\n-\tconst char *editor = getenv(\"GIT_SEQUENCE_EDITOR\");\n-\n-\tif (!editor)\n-\t\tgit_config_get_string_const(\"sequence.editor\", &editor);\n-\tif (!editor)\n-\t\teditor = git_editor();\n-\n-\treturn editor;\n+\treturn git_editor(\"SEQUENCE\");\n }\n \n static int launch_specified_editor(const char *editor, const char *path,\n@@ -118,7 +129,7 @@ static int launch_specified_editor(const char *editor, const char *path,\n \n int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n {\n-\treturn launch_specified_editor(git_editor(), path, buffer, env);\n+\treturn launch_specified_editor(git_editor(NULL), path, buffer, env);\n }\n \n int launch_sequence_editor(const char *path, struct strbuf *buffer,\n"},{"id":"397555","messageId":"20200511154226.GC1415@coredump.intra.peff.net","threadId":"53437","inReplyTo":"xmqqr1vqscuy.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-11T15:42:26Z","receivedAt":"2020-05-11T15:42:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 11, 2020 at 08:36:05AM -0700, Junio C Hamano wrote:\n\n> > So I'm OK to leave the status quo and let people use the GIT_EDITOR\n> > solution in this instance. But I'd also be happy to take a patch for\n> > \"--no-editor\" or similar if somebody wants to work it up.\n> \n> I actually would support --no-editor.  One thing nobody noticed so\n> far is that \"git-jump\" is only compatible with editors that support\n> the \"-q\" option from the command line, and \"cat\" is not among them.\n\nOh, good point. GIT_EDITOR='cat -- 2>/dev/null' works, but is rather\nobscure. :)\n\n> Another thing I was thinking about was a change like the attached.\n> Plugging it thru \"git var\" to allow \"git var GIT_JUMP_EDITOR\" may\n> allow vim users to set it to 'cat' while setting GIT_EDITOR to vim.\n\nI wouldn't use it myself, but I don't have any objection to adding\nsupport.\n\n>  cache.h  |  2 +-\n>  editor.c | 33 ++++++++++++++++++++++-----------\n\nYou'd presumably need to make git-var understand that GIT_JUMP_EDITOR\nshould fall back to GIT_EDITOR.\n\n-Peff\n"},{"id":"397557","messageId":"xmqqy2pyqv11.fsf_-_@gitster.c.googlers.com","threadId":"53437","inReplyTo":"20200511154226.GC1415@coredump.intra.peff.net","subject":"Re* [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-11T16:46:34Z","receivedAt":"2020-05-11T16:46:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, May 11, 2020 at 08:36:05AM -0700, Junio C Hamano wrote:\n>\n>> > So I'm OK to leave the status quo and let people use the GIT_EDITOR\n>> > solution in this instance. But I'd also be happy to take a patch for\n>> > \"--no-editor\" or similar if somebody wants to work it up.\n>> \n>> I actually would support --no-editor.  One thing nobody noticed so\n>> far is that \"git-jump\" is only compatible with editors that support\n>> the \"-q\" option from the command line, and \"cat\" is not among them.\n>\n> Oh, good point. GIT_EDITOR='cat -- 2>/dev/null' works, but is rather\n> obscure. :)\n\nLest we all forget...\n\n-- >8 --\nSubject: git-jump: just show the list with the \"--no-editor\" option\n\nThe \"git jump\" script (in contrib/) creates a list of interesting\nplaces to be visited in an editor, and then open the editor to visit\nthe place.  Some editors, however, can read the list directly and\nuse it to visit these places (e.g. vim's quickfix list, or emacs's\nfind-grep/compilation buffer) and do not want \"git jump\" to invoke\na separate editor.\n\nUsers can _almost_ do this already by setting GIT_EDITOR to \"cat\",\nexcept that \"git jump\" assumes that the editor it spawns support a\n\"-q\" option from the command line, and unfortunately \"cat\" is not\namong such editors.\n\nTeach it the \"--no-editor\" option, which tells the command to show\nthe list it generated to its standard output.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n contrib/git-jump/git-jump | 21 +++++++++++++++++++--\n 1 file changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 931b0fe3a9..26a7159053 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -2,7 +2,7 @@\n \n usage() {\n \tcat <<\\EOF\n-usage: git jump <mode> [<args>]\n+usage: git jump [--no-editor] <mode> [<args>]\n \n Jump to interesting elements in an editor.\n The <mode> parameter is one of:\n@@ -64,6 +64,18 @@ mode_ws() {\n \tgit diff --check \"$@\"\n }\n \n+edit=yes\n+\n+while\tcase \"$#,$1\" in\n+\t0,*) break ;;\n+\t*,--no-editor) edit=no ;;\n+\t*,--*) usage >&2; exit 1 ;;\n+\t*) break ;;\n+\tesac\n+do\n+\tshift\n+done\n+\n if test $# -lt 1; then\n \tusage >&2\n \texit 1\n@@ -75,4 +87,9 @@ tmp=`mktemp -t git-jump.XXXXXX` || exit 1\n type \"mode_$mode\" >/dev/null 2>&1 || { usage >&2; exit 1; }\n \"mode_$mode\" \"$@\" >\"$tmp\"\n test -s \"$tmp\" || exit 0\n-open_editor \"$tmp\"\n+\n+case \"$edit\" in\n+yes)\topen_editor \"$tmp\" ;;\n+no)\tcat \"$tmp\" ;;\n+esac\n+\n"},{"id":"397621","messageId":"20200512192323.GC54565@coredump.intra.peff.net","threadId":"53437","inReplyTo":"xmqqy2pyqv11.fsf_-_@gitster.c.googlers.com","subject":"Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-12T19:23:23Z","receivedAt":"2020-05-12T19:23:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 11, 2020 at 09:46:34AM -0700, Junio C Hamano wrote:\n\n> Lest we all forget...\n> \n> -- >8 --\n> Subject: git-jump: just show the list with the \"--no-editor\" option\n\nThanks for tying this up. It seems to work as advertised. A few nits:\n\n> +edit=yes\n> +\n> +while\tcase \"$#,$1\" in\n\nTab between \"while\" and \"case\"?\n\n> +\t0,*) break ;;\n> +\t*,--no-editor) edit=no ;;\n> +\t*,--*) usage >&2; exit 1 ;;\n> +\t*) break ;;\n> +\tesac\n> +do\n> +\tshift\n> +done\n\nI found the use of \"case\" in the loop conditional a little unusual. I'd\nhave probably written:\n\n  while test $# -gt 0\n  do\n\tcase \"$1\" in\n\t--no-editor) edit=no ;;\n\t--*) usage >&2; exit 1 ;;\n\t*) break ;;\n\tesac\n\tshift\n  done\n\n> @@ -75,4 +87,9 @@ tmp=`mktemp -t git-jump.XXXXXX` || exit 1\n>  type \"mode_$mode\" >/dev/null 2>&1 || { usage >&2; exit 1; }\n>  \"mode_$mode\" \"$@\" >\"$tmp\"\n>  test -s \"$tmp\" || exit 0\n> -open_editor \"$tmp\"\n> +\n> +case \"$edit\" in\n> +yes)\topen_editor \"$tmp\" ;;\n> +no)\tcat \"$tmp\" ;;\n> +esac\n> +\n\n\"diff --check\" complains about the empty line.\n\nIt probably doesn't matter much, but we could skip the tempfile entirely\nin no-editor mode. I.e.:\n\n  if test \"$edit\" = \"no\"\n  then\n    \"mode_$mode\" \"$@\"\n  fi\n\n  # otherwise set up trap, mktemp, etc\n\n-Peff\n"},{"id":"397633","messageId":"xmqqy2pwalj9.fsf@gitster.c.googlers.com","threadId":"53437","inReplyTo":"20200512192323.GC54565@coredump.intra.peff.net","subject":"Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-12T21:30:34Z","receivedAt":"2020-05-12T21:30:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, May 11, 2020 at 09:46:34AM -0700, Junio C Hamano wrote:\n>\n>> Lest we all forget...\n>> \n>> -- >8 --\n>> Subject: git-jump: just show the list with the \"--no-editor\" option\n>\n> Thanks for tying this up. It seems to work as advertised. A few nits:\n>\n>> +edit=yes\n>> +\n>> +while\tcase \"$#,$1\" in\n>\n> Tab between \"while\" and \"case\"?\n\nYup.  Just to align case and its arms.\n\n>> +\t0,*) break ;;\n>> +\t*,--no-editor) edit=no ;;\n>> +\t*,--*) usage >&2; exit 1 ;;\n>> +\t*) break ;;\n>> +\tesac\n>> +do\n>> +\tshift\n>> +done\n>\n> I found the use of \"case\" in the loop conditional a little unusual.\n\nIt's pretty-much personal preference, I think.  I could replace\ns/break/false/ if you find it easier to understand.\n\n> It probably doesn't matter much, but we could skip the tempfile entirely\n> in no-editor mode. I.e.:\n>\n>   if test \"$edit\" = \"no\"\n>   then\n>     \"mode_$mode\" \"$@\"\n>   fi\n>\n>   # otherwise set up trap, mktemp, etc\n\nMakes a lot of sense to me.\n\n"},{"id":"397699","messageId":"20200513045238.GB65110@coredump.intra.peff.net","threadId":"53437","inReplyTo":"xmqqy2pwalj9.fsf@gitster.c.googlers.com","subject":"Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-13T04:52:38Z","receivedAt":"2020-05-13T04:52:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 12, 2020 at 02:30:34PM -0700, Junio C Hamano wrote:\n\n> >> +edit=yes\n> >> +\n> >> +while\tcase \"$#,$1\" in\n> >\n> > Tab between \"while\" and \"case\"?\n> \n> Yup.  Just to align case and its arms.\n\nI guess that makes sense, though I'd probably have used spaces to do so.\nHaving a tab in the middle of the line is unusual.\n\n> >> +\t0,*) break ;;\n> >> +\t*,--no-editor) edit=no ;;\n> >> +\t*,--*) usage >&2; exit 1 ;;\n> >> +\t*) break ;;\n> >> +\tesac\n> >> +do\n> >> +\tshift\n> >> +done\n> >\n> > I found the use of \"case\" in the loop conditional a little unusual.\n> \n> It's pretty-much personal preference, I think.  I could replace\n> s/break/false/ if you find it easier to understand.\n\nI think the part that most threw me off is looking at the arg-count in\neach case arm. It's \"*\" in most, which really means \"do not bother to\nlook at it\" (which I think is why I found a loop condition on \"$# -gt 0\"\nto be more natural).\n\nI can live with it either way.\n\n-Peff\n"}]}