* Re: [PATCH] contrib/git-jump: cat output when not a terminal
@ 2020-05-10 20:26 Benjamin
2020-05-11 14:33 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Benjamin @ 2020-05-10 20:26 UTC (permalink / raw)
To: gitster; +Cc: 321.george, git, peff
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 20:26 [PATCH] contrib/git-jump: cat output when not a terminal Benjamin @ 2020-05-11 14:33 ` Junio C Hamano 0 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2020-05-11 14:33 UTC (permalink / raw) To: Benjamin; +Cc: 321.george, git, peff 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". ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] contrib/git-jump: cat output when not a terminal @ 2020-05-09 19:15 George Brown 2020-05-09 21:41 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: George Brown @ 2020-05-09 19:15 UTC (permalink / raw) To: git; +Cc: peff 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() { ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-09 19:15 George Brown @ 2020-05-09 21:41 ` Junio C Hamano 2020-05-09 22:04 ` George Brown 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2020-05-09 21:41 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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() { ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-09 21:41 ` Junio C Hamano @ 2020-05-09 22:04 ` George Brown 2020-05-09 23:42 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: George Brown @ 2020-05-09 22:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, peff > 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() { ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-09 22:04 ` George Brown @ 2020-05-09 23:42 ` Junio C Hamano 2020-05-10 9:03 ` George Brown 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2020-05-09 23:42 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-09 23:42 ` Junio C Hamano @ 2020-05-10 9:03 ` George Brown 2020-05-10 16:47 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: George Brown @ 2020-05-10 9:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, peff > 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 9:03 ` George Brown @ 2020-05-10 16:47 ` Junio C Hamano 2020-05-10 17:33 ` George Brown 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2020-05-10 16:47 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 16:47 ` Junio C Hamano @ 2020-05-10 17:33 ` George Brown 2020-05-10 18:12 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: George Brown @ 2020-05-10 17:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, peff > 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 17:33 ` George Brown @ 2020-05-10 18:12 ` Junio C Hamano 2020-05-10 18:34 ` George Brown 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2020-05-10 18:12 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 18:12 ` Junio C Hamano @ 2020-05-10 18:34 ` George Brown 2020-05-10 19:10 ` Junio C Hamano 2020-05-11 14:31 ` Jeff King 0 siblings, 2 replies; 19+ messages in thread From: George Brown @ 2020-05-10 18:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, peff 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 18:34 ` George Brown @ 2020-05-10 19:10 ` Junio C Hamano 2020-05-10 19:25 ` George Brown 2020-05-10 19:38 ` Junio C Hamano 2020-05-11 14:31 ` Jeff King 1 sibling, 2 replies; 19+ messages in thread From: Junio C Hamano @ 2020-05-10 19:10 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 19:10 ` Junio C Hamano @ 2020-05-10 19:25 ` George Brown 2020-05-10 19:38 ` Junio C Hamano 1 sibling, 0 replies; 19+ messages in thread From: George Brown @ 2020-05-10 19:25 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, peff > 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 19:10 ` Junio C Hamano 2020-05-10 19:25 ` George Brown @ 2020-05-10 19:38 ` Junio C Hamano 2020-05-10 20:20 ` George Brown 1 sibling, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2020-05-10 19:38 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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 ;-) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 19:38 ` Junio C Hamano @ 2020-05-10 20:20 ` George Brown 2020-05-11 14:31 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: George Brown @ 2020-05-10 20:20 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, peff > 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 20:20 ` George Brown @ 2020-05-11 14:31 ` Junio C Hamano 0 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2020-05-11 14:31 UTC (permalink / raw) To: George Brown; +Cc: git, peff 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-10 18:34 ` George Brown 2020-05-10 19:10 ` Junio C Hamano @ 2020-05-11 14:31 ` Jeff King 2020-05-11 15:36 ` Junio C Hamano 1 sibling, 1 reply; 19+ messages in thread From: Jeff King @ 2020-05-11 14:31 UTC (permalink / raw) To: George Brown; +Cc: Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-11 14:31 ` Jeff King @ 2020-05-11 15:36 ` Junio C Hamano 2020-05-11 15:42 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2020-05-11 15:36 UTC (permalink / raw) To: Jeff King; +Cc: George Brown, git 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, ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] contrib/git-jump: cat output when not a terminal 2020-05-11 15:36 ` Junio C Hamano @ 2020-05-11 15:42 ` Jeff King 0 siblings, 0 replies; 19+ messages in thread From: Jeff King @ 2020-05-11 15:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: George Brown, git 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-05-11 15:42 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-10 20:26 [PATCH] contrib/git-jump: cat output when not a terminal Benjamin 2020-05-11 14:33 ` Junio C Hamano -- strict thread matches above, loose matches on Subject: below -- 2020-05-09 19:15 George Brown 2020-05-09 21:41 ` Junio C Hamano 2020-05-09 22:04 ` George Brown 2020-05-09 23:42 ` Junio C Hamano 2020-05-10 9:03 ` George Brown 2020-05-10 16:47 ` Junio C Hamano 2020-05-10 17:33 ` George Brown 2020-05-10 18:12 ` Junio C Hamano 2020-05-10 18:34 ` George Brown 2020-05-10 19:10 ` Junio C Hamano 2020-05-10 19:25 ` George Brown 2020-05-10 19:38 ` Junio C Hamano 2020-05-10 20:20 ` George Brown 2020-05-11 14:31 ` Junio C Hamano 2020-05-11 14:31 ` Jeff King 2020-05-11 15:36 ` Junio C Hamano 2020-05-11 15:42 ` Jeff King
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).