[RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i

[RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i

[Peter Oberndorfer]

If rebase.editor is not set interactive rebase falls back
to the default editor.

With this change is it possible to have a separate
(possibly graphical) editor that helps the user
during a interactive rebase.

Using $GIT_EDITOR or core.editor config var for this is not possible
since it is also used to start the commit message editor for reword action.

Signed-off-by: Peter Oberndorfer <kumbayo84@arcor.de>
---
Hi,

i wrote a (not yet released) git rebase -i helper that allows to order commits
by drag/drop and allows to select the action from a combo box.
(written in Qt)
See http://i55.tinypic.com/2d94gg0.jpg for how it currently looks like. :-)
No more typos, no more lost commit by cutting without pasting...

To integrate this properly into git i need something like this patch.

Open questions/problems:
* GIT_EDITOR env var is not honored anymore after this change.
  Help from somebody with more bash knowledge is highly appreciated!

* Should git_rebase_editor be in git-rebase--interactive.sh instead
  (since it is only used there)

* How should the config be called?
  It is not directly used during rebase, only during rebase -i
  that might not be fully clear from the config name.

* Better config.txt description?

Thanks,
Greetings Peter

PS: My tool will hopefully be released soon.
Cleanup code, test(lin/ win), write some doc (how to use with git),
choose name :-), choose license...

 Documentation/config.txt   |    6 ++++++
 git-rebase--interactive.sh |    2 +-
 git-sh-setup.sh            |   13 +++++++++++++
 3 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 03296b7..1d9ae79 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1591,6 +1591,12 @@ rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
 	rebase. False by default.
 
+rebase.editor::
+	Text editor used by git rebase -i for editing the rebasse todo file.
+	The value is meant to be interpreted by the shell when it is used.
+	When not configured the default commit message editor is used instead.
+	See "core.editor"
+
 rebase.autosquash::
 	If set to true enable '--autosquash' option by default.
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94f36c2..0f3b569 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -832,7 +832,7 @@ has_action "$todo" ||
 	die_abort "Nothing to do"
 
 cp "$todo" "$todo".backup
-git_editor "$todo" ||
+git_rebase_editor "$todo" ||
 	die_abort "Could not execute editor"
 
 has_action "$todo" ||
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 8e427da..303fb96 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -113,6 +113,19 @@ git_editor() {
 	eval "$GIT_EDITOR" '"$@"'
 }
 
+git_rebase_editor() {
+	if test -z "${GIT_REBASEI_EDITOR:+set}"
+	then
+		GIT_REBASEI_EDITOR="$(git config rebase.editor)"
+		if [ -z "$GIT_REBASEI_EDITOR" ]
+		then
+			GIT_REBASEI_EDITOR="$(git var GIT_EDITOR)" || return $?
+		fi
+	fi
+
+	eval "$GIT_REBASEI_EDITOR" '"$@"'
+}
+
 git_pager() {
 	if test -t 1
 	then
-- 
1.7.7.215.gfef80

Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i

[Phil Hord]

On Tue, Oct 11, 2011 at 1:56 PM, Peter Oberndorfer <kumbayo84@arcor.de> wrote:
> i wrote a (not yet released) git rebase -i helper that allows to order commits
> by drag/drop and allows to select the action from a combo box.
> (written in Qt)
> See http://i55.tinypic.com/2d94gg0.jpg for how it currently looks like. :-)
> No more typos, no more lost commit by cutting without pasting...

[+1]

>
> To integrate this properly into git i need something like this patch.
>
> Open questions/problems:
> * GIT_EDITOR env var is not honored anymore after this change.
>  Help from somebody with more bash knowledge is highly appreciated!
>
> * Should git_rebase_editor be in git-rebase--interactive.sh instead
>  (since it is only used there)
>
> * How should the config be called?
>  It is not directly used during rebase, only during rebase -i
>  that might not be fully clear from the config name.
>
> * Better config.txt description?
>
> Thanks,
> Greetings Peter
>
> PS: My tool will hopefully be released soon.
> Cleanup code, test(lin/ win), write some doc (how to use with git),
> choose name :-), choose license...
>
>  Documentation/config.txt   |    6 ++++++
>  git-rebase--interactive.sh |    2 +-
>  git-sh-setup.sh            |   13 +++++++++++++
>  3 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 03296b7..1d9ae79 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1591,6 +1591,12 @@ rebase.stat::
>        Whether to show a diffstat of what changed upstream since the last
>        rebase. False by default.
>
> +rebase.editor::
> +       Text editor used by git rebase -i for editing the rebasse todo file.
s/rebasse/rebase/

>  cp "$todo" "$todo".backup
> -git_editor "$todo" ||
> +git_rebase_editor "$todo" ||
>        die_abort "Could not execute editor"

Maybe something like this would work:
  git_rebase_editor "$todo" ||
  git_editor "$todo" ||
       die_abort "Could not execute editor"

If git_rebase_editor call returns an error (non-zero exit code), then
git_editor will be invoked. If that also returns an error, then the
die_abort is called.

I think this will allow your env:GIT_EDITOR to work as expected.

Phil

Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i

[Junio C Hamano]

Peter Oberndorfer <kumbayo84@arcor.de> writes:

> Using $GIT_EDITOR or core.editor config var for this is not possible
> since it is also used to start the commit message editor for reword action.

Your tool _could_ be smart about this issue and inspect the contents to
launch a real editor when it is fed a material not for sequencing, but
that feels hacky.

> * GIT_EDITOR env var is not honored anymore after this change.

Care to explain?  "git var" knows magic about a few variables like
GIT_EDITOR and GIT_PAGER.

	$ git config core.editor vim
	$ GIT_EDITOR=vi EDITOR=emacs git var GIT_EDITOR
        vi
	$ unset GIT_EDITOR; EDITOR=emacs git var GIT_EDITOR
        emacs

> * Should git_rebase_editor be in git-rebase--interactive.sh instead

Probably yes.

> * How should the config be called?

Given that in the longer term we would be using a unified sequencer
machinery for not just rebase-i but for am and cherry-pick, I would advise
against calling this anything "rebase".  How does "sequence.edit" sound?

You need to be prepared to adjust your code to deal with new kinds of
sequencing insns in the insn sheet and possibly a format change of the
insn sheet itself.

Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i

[Peter Oberndorfer]

On Dienstag, 11. Oktober 2011, Junio C Hamano wrote:
> Peter Oberndorfer <kumbayo84@arcor.de> writes:
> 
> > Using $GIT_EDITOR or core.editor config var for this is not possible
> > since it is also used to start the commit message editor for reword action.
> 
> Your tool _could_ be smart about this issue and inspect the contents to
> launch a real editor when it is fed a material not for sequencing, but
> that feels hacky.

I already tried this, but my first version did not redirect stdin/stdout
so vi stayed in background and the whole thing just hung.
I did not try further because i assumed more problems would appear
when redirecting stdin/stdout...

> > * GIT_EDITOR env var is not honored anymore after this change.
> 
> Care to explain?  "git var" knows magic about a few variables like
> GIT_EDITOR and GIT_PAGER.
> 
> 	$ git config core.editor vim
> 	$ GIT_EDITOR=vi EDITOR=emacs git var GIT_EDITOR
>         vi
> 	$ unset GIT_EDITOR; EDITOR=emacs git var GIT_EDITOR
>         emacs

Sorry i was wrong, i missed that git var looks at $GIT_EDITOR.

So the sequence for choosing the sequencer editor is:
$GIT_SEQUENCE_EDITOR
config sequence.editor
var GIT_EDITOR

Which looks OK to me.

> > * Should git_rebase_editor be in git-rebase--interactive.sh instead
> 
> Probably yes.

OK, will do.

> 
> > * How should the config be called?
> 
> Given that in the longer term we would be using a unified sequencer
> machinery for not just rebase-i but for am and cherry-pick, I would advise
> against calling this anything "rebase".  How does "sequence.edit" sound?
> 

I do not really care very much, but how about sequence.editor?
Sounds more similar to core.editor

> You need to be prepared to adjust your code to deal with new kinds of
> sequencing insns in the insn sheet and possibly a format change of the
> insn sheet itself.

I assume instruction sheet is the commented out part that looks like:
# Commands:
#  p, pick = use commit
#  r, reword = use commit, but edit the commit message

Currently all lines starting with # are ignored.
(They are also not written to the output when finished
which is a point I might have to change...)

Also the instructions are currently not taken from this instruction sheet.
They are all hardcoded.

Thanks for the feedback
Greetings Peter