rebase -i: print the editor name if it fails to launch
PATCH 1/1 rebase -i: print the editor name if it fails to launch
The "git rebase -i" commands simply prints "Could not execute editor" if the editor fails to launch.
Be a little bit more helpful and be consistent with the built-in commands that use launch_editor() and also print the name of the editor that failed to launch.
---
Here is a suggestion for how "git rebase -i" can be a little bit more helpful if it fails to launch the editor. This patch can be applied on top of my previous patch.
I am not sure whether "git am" also should be modified to call git_editor_error_string. Currently it ignores any errors from the call to git_editor. Should it abort if there is an error or should it print a warning like this
echo "Warning: $(git_editor_error_string)"
if the editor fails to launch?
Grepping all *.sh files, I found no other references to git_editor.
git-rebase--interactive.sh | 3 +-- git-sh-setup.sh | 9 +++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)
git-rebase--interactive.sh +1 -2
@@ -778,8 +778,7 @@ EOF die_abort "Nothing to do" cp "$TODO" "$TODO".backup - git_editor "$TODO" || - die_abort "Could not execute editor" + git_editor "$TODO" || die_abort "$(git_editor_error_string)" has_action "$TODO" || die_abort "Nothing to do"
git-sh-setup.sh +9 -0
@@ -107,6 +107,15 @@ git_editor() { eval "$GIT_EDITOR" '"$@"' } +git_editor_error_string() { + if test -z "${GIT_EDITOR:+set}" + then + GIT_EDITOR="$(git var GIT_EDITOR)" || + (echo "No editor configured."; return 0) + fi + echo "There was a problem with the editor '$GIT_EDITOR'." +} + sane_grep () { GREP_OPTIONS= LC_ALL=C grep "$@" }
Signature · 1 line
1.6.6.rc3.2.ge3899
Oops, I seem to have hit the Send button too early. Please ignore the first email that stops in mid sentence.
Signature · 1 line
Björn Gustavsson, Erlang/OTP, Ericsson AB
Quoted 8 lines · Björn Gustavsson
I am not sure whether "git am" also should be modified to call
git_editor_error_string. Currently it ignores any errors from the call togit_editor. Should it abort if there is an error or should it print a warning like thisecho "Warning: $(git_editor_error_string)"
if the editor fails to launch?
It probably is better, even though the lack of check there is not as harmful compared to the one in "rebase -i" that annoyed you (which I also see occasionally after freshly rebooting my environment, as I use emacsclient like you do). You will be taken back to the "what now?" interactive loop and have a chance to \C-c out of it. So such a patch would help not in the sense that it would prevent the command from doing what you didn't intend to (which is your "rebase -i" patch is about), but in the sense that it hints what needs to be fixed once you break out of the command.
git-rebase--interactive.sh EOF · line 781
Quoted 3 lines
git-rebase--interactive.sh +1
-2- git_editor "$TODO" || - die_abort "Could not execute editor" + git_editor "$TODO" || die_abort "$(git_editor_error_string)"
Isn't this too elaborate? git_editor() has already run and when it attempted to launch the editor it assigned to GIT_EDITOR in order to use it as an eval string.
git_editor "$TODO" ||
die_abort "Failed to run '${GIT_EDITOR:-your editor}'"would suffice, no?
The alternative string is for a case where "git var" gave an empty string, or GIT_EDITOR was an empty string from the beginning.
2009/12/20 Junio C Hamano <[email protected]>:
Quoted 3 lines
Isn't this too elaborate? git_editor() has already run and when it attempted to launch the editor it assigned to
GIT_EDITORin order to use it as an eval string.
I wanted to hide the details about the inner working of git_editor(), but since it is only called in one place, yes, it is too elaborate.
Quoted 5 lines
git_editor "$TODO" ||
die_abort "Failed to run '${GIT_EDITOR:-your editor}'"would suffice, no?
Yes. Much better. I will rewrite my patch when I'll get home from work later today.
Signature · 1 line
Björn Gustavsson, Erlang/OTP, Ericsson AB