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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



George Brown <321.george@xxxxxxxxx> 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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux