[PATCH] Added giteditor script to show diff while editing commit message.

[PATCH] Added giteditor script to show diff while editing commit message.

[ted]

From: Ted Pavlic <ted@tedpavlic.com>

This new script (contrib/giteditor/giteditor) is an example GIT_EDITOR
that causes the editor to open the commit message as well as a "git diff
--cached". As a result, a window showing a diff of what is being
committed is opened alongside the commit message.

This script also detects when "stg edit" is being called and uses "stg
show" instead.

This script is highly influenced by the "hgeditor" script distributed
with the Mercurial SCM.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
---
 contrib/giteditor/README    |    9 ++++
 contrib/giteditor/giteditor |  111 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 0 deletions(-)
 create mode 100644 contrib/giteditor/README
 create mode 100755 contrib/giteditor/giteditor

diff --git a/contrib/giteditor/README b/contrib/giteditor/README
new file mode 100644
index 0000000..b769c3e
--- /dev/null
+++ b/contrib/giteditor/README
@@ -0,0 +1,9 @@
+A GIT_EDITOR to show diff alongside commit message. User can review diff
+within commit edit window. Works with StGit ("stg edit") as well.
+
+To use this script, set it as the value of GIT_EDITOR (or core.editor).
+
+
+Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
+Highly influenced by hgeditor script distributed with Mercurial SCM.
+Distributed under the GNU General Public License, version 2.0.
diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
new file mode 100755
index 0000000..13ca5f6
--- /dev/null
+++ b/contrib/giteditor/giteditor
@@ -0,0 +1,111 @@
+#!/bin/sh
+#
+# A GIT_EDITOR to show diff alongside commit message. User can review
+# diff within commit edit window. Works with StGit ("stg edit") as well.
+#
+# Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
+# Highly influenced by hgeditor script distributed with Mercurial SCM.
+# Distributed under the GNU General Public License, version 2.0.
+#
+# To use this script, set it as the value of GIT_EDITOR (or
+# core.editor).
+#
+
+# Find git
+[ -z "${GIT}" ] && GIT="git"
+
+# Find stg
+[ -z "${STG}" ] && STG="stg"
+
+# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
+EDITOR=${GIT_EDITOR_EDITOR} || \
+    EDITOR=${VISUAL} || \
+    EDITOR=${EDITOR} || \
+    EDITOR="vi";
+
+# If we recognize a popular editor, add necessary flags
+case "${EDITOR}" in
+    emacs)
+        EDITOR="${EDITOR} -nw"
+        ;;
+    mvim|gvim|vim)
+        EDITOR="${EDITOR} -f -o"
+        ;;
+esac
+
+# Remove temporary files even if we get interrupted
+GITTMP=""
+cleanup_exit() { 
+    [ -n "${GITTMP}" ] && rm -rf "${GITTMP}" 
+}
+trap "cleanup_exit" 0 # normal exit
+trap "exit 255" 1 2 3 6 15 # HUP INT QUIT ABRT TERM
+
+# End GITTMP in ".git" so that "*.git/" syntax highlighting recognition
+# doesn't break
+GITTMP="${TMPDIR-/tmp}/giteditor.$RANDOM.$RANDOM.$RANDOM.$$.git"
+(umask 077 && mkdir "${GITTMP}") || {
+    echo "Could not create temporary directory! Exiting." 1>&2
+    exit 1
+}
+
+# For git, COMMITMSG=COMMIT_EDITMSG
+# For stg, COMMITMSG=.stgit-edit.txt
+# etc.
+COMMITMSG=$( basename "$1" )
+
+case "${COMMITMSG}" in
+    .stgit-edit.txt) 
+        DIFFCMD="${STG}"
+        DIFFARGS="show"
+        ;;
+    *) 
+        DIFFCMD="${GIT}"
+        DIFFARGS="diff --cached"
+        ;;
+esac
+
+if [ -f "$1" ]; then
+    # We were passed an existing commit message
+
+    "${DIFFCMD}" ${DIFFARGS} >> "${GITTMP}/diff"
+## Uncomment if you only want to see diff of what changed
+## (note that it only works if DIFFCMD is git)
+#    (
+#        grep '^#.*modified:' "$1" | cut -b 15- | while read changed; do
+#            "${DIFFCMD}" ${DIFFARGS} "${changed}" >> "${GITTMP}/diff"
+#        done
+#    )
+
+     cat "$1" > "${GITTMP}/${COMMITMSG}"
+
+else
+
+    # Give us a blank COMMITMSG to edit
+    touch "${GITTMP}/${COMMITMSG}"
+
+    # Generate the diff
+    "${DIFFCMD}" ${DIFFARGS} >> "${GITTMP}/diff"
+    #touch "${GITTMP}/diff"
+
+fi
+
+# Use MD5 to see if commit message changed (necessary?)
+MD5=$(which md5sum 2>/dev/null) || \
+    MD5=$(which md5 2>/dev/null)
+
+[ -x "${MD5}" ] && CHECKSUM=$( ${MD5} "${GITTMP}/${COMMITMSG}" )
+if [ -s "${GITTMP}/diff" ]; then
+    # Diff is non-empty, so edit msg and diff
+    ${EDITOR} "${GITTMP}/${COMMITMSG}" "${GITTMP}/diff" || exit $?
+else
+    # Empty diff. Only edit msg
+    ${EDITOR} "${GITTMP}/${COMMITMSG}" || exit $?
+fi
+[ -x "${MD5}" ] && (echo "${CHECKSUM}" | ${MD5} -c >/dev/null 2>&1 && exit 13)
+
+# Commit message changed, so dump it on original message from Git
+mv "${GITTMP}/${COMMITMSG}" "$1"
+
+# (recall that GITTMP directory gets cleaned up by trap above)
+exit $?
-- 
1.6.1.213.g28da8

Re: [PATCH] Added giteditor script to show diff while editing commit message.

[Ted Pavlic]

>> This script also detects when "stg edit" is being called and uses "stg
>> show" instead.
> You do know about "git commit -v", right? (Although that displays
> commit message and diff in a single buffer, not two separate ones like
> your script does.)

Actually, I didn't. I was looking for something like hgeditor for git, 
and I found something on vim.org that almost hit the mark but no mention 
of "commit -v".

That being said, I like having the diff up in a separate window. Plus, 
"git commit -v" doesn't help me when I want to "stg edit".

Thanks --
Ted

-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

Re: [PATCH] Added giteditor script to show diff while editing commit message.

[Johannes Schindelin]

Hi,

the subject could use some work.  For example, I would prefix it with 
"contrib:", and -- imitating other commit messages -- use the imperative 
form "Add" instead of the frowned-upon past tense.

On Wed, 21 Jan 2009, ted@tedpavlic.com wrote:

> From: Ted Pavlic <ted@tedpavlic.com>

As this is exactly what your email said in its header, it is redundant 
information.  Worse, it is information that made me look back to know why 
it needs to be there.  Distracting.

> diff --git a/contrib/giteditor/README b/contrib/giteditor/README
> new file mode 100644
> index 0000000..b769c3e
> --- /dev/null
> +++ b/contrib/giteditor/README
> @@ -0,0 +1,9 @@
> +A GIT_EDITOR to show diff alongside commit message. User can review diff
> +within commit edit window. Works with StGit ("stg edit") as well.
> +
> +To use this script, set it as the value of GIT_EDITOR (or core.editor).
> +
> +
> +Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
> +Highly influenced by hgeditor script distributed with Mercurial SCM.
> +Distributed under the GNU General Public License, version 2.0.

What information does the README add that is not in the script itself?

If there is none, please refrain from adding the README to begin with.

> diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
> new file mode 100755
> index 0000000..13ca5f6
> --- /dev/null
> +++ b/contrib/giteditor/giteditor
> @@ -0,0 +1,111 @@
> +#!/bin/sh
> +#
> +# A GIT_EDITOR to show diff alongside commit message.

Maybe "Set GIT_EDITOR to giteditor if you want to see a diff of what will 
be committed in the editor"?

> +# Find git
> +[ -z "${GIT}" ] && GIT="git"

Yes, I know it is contrib/, but you may want to adopt Git's coding style 
early.

Besides, I find it funny that you want to override git with $GIT.

> +# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
> +EDITOR=${GIT_EDITOR_EDITOR} || \
> +    EDITOR=${VISUAL} || \
> +    EDITOR=${EDITOR} || \
> +    EDITOR="vi";
> +
> +# If we recognize a popular editor, add necessary flags
> +case "${EDITOR}" in
> +    emacs)
> +        EDITOR="${EDITOR} -nw"

Mhm.  Should this not be the user's choice?  Some like emacs to start up 
in a window.

> +# Remove temporary files even if we get interrupted
> +GITTMP=""

GITTMP= would be completely sufficient.  Not to mention the consistency 
with Git's shell code.

> +# End GITTMP in ".git" so that "*.git/" syntax highlighting recognition
> +# doesn't break
> +GITTMP="${TMPDIR-/tmp}/giteditor.$RANDOM.$RANDOM.$RANDOM.$$.git"
> +(umask 077 && mkdir "${GITTMP}") || {
> +    echo "Could not create temporary directory! Exiting." 1>&2
> +    exit 1
> +}

Umm.  Why?  Why do you need a temporary .git directory?

> +if [ -f "$1" ]; then
> +    # We were passed an existing commit message
> +
> +    "${DIFFCMD}" ${DIFFARGS} >> "${GITTMP}/diff"
> +## Uncomment if you only want to see diff of what changed
> +## (note that it only works if DIFFCMD is git)
> +#    (
> +#        grep '^#.*modified:' "$1" | cut -b 15- | while read changed; do
> +#            "${DIFFCMD}" ${DIFFARGS} "${changed}" >> "${GITTMP}/diff"
> +#        done
> +#    )

--diff-filter=M

> +
> +     cat "$1" > "${GITTMP}/${COMMITMSG}"
> +
> +else
> +
> +    # Give us a blank COMMITMSG to edit
> +    touch "${GITTMP}/${COMMITMSG}"

Why not just touch it, instead of testing if the file exists first?

> +
> +    # Generate the diff
> +    "${DIFFCMD}" ${DIFFARGS} >> "${GITTMP}/diff"
> +    #touch "${GITTMP}/diff"

Commented out code in a submitted patch?

> +
> +fi
> +
> +# Use MD5 to see if commit message changed (necessary?)
> +MD5=$(which md5sum 2>/dev/null) || \
> +    MD5=$(which md5 2>/dev/null)
> +
> +[ -x "${MD5}" ] && CHECKSUM=$( ${MD5} "${GITTMP}/${COMMITMSG}" )
> +if [ -s "${GITTMP}/diff" ]; then
> +    # Diff is non-empty, so edit msg and diff
> +    ${EDITOR} "${GITTMP}/${COMMITMSG}" "${GITTMP}/diff" || exit $?

vi users will hate you, as you do not give them a chance to edit the 
message after having seen the diff.

> +else
> +    # Empty diff. Only edit msg
> +    ${EDITOR} "${GITTMP}/${COMMITMSG}" || exit $?
> +fi
> +[ -x "${MD5}" ] && (echo "${CHECKSUM}" | ${MD5} -c >/dev/null 2>&1 && exit 13)

git commit will abort anyway if the commit message has not changed.  Plus, 
it does a better job, as it checks only the non-commented-out text.

BTW why on earth do you put every single variable name in curly brackets?

> +
> +# Commit message changed, so dump it on original message from Git
> +mv "${GITTMP}/${COMMITMSG}" "$1"

And why did you not use "$1" all the time?

> +
> +# (recall that GITTMP directory gets cleaned up by trap above)
> +exit $?

Just writing "exit" is the same in effect, but preferred in Git shell 
coding.

Besides all that criticism, there is also a fundamental issue.  The diff 
is in a separate file.

Instead, I suggest having something like this:

-- snip --
#!/bin/sh

# set GIT_EDITOR or core.editor to this script if you want to see a diff
# instead of the output of "git status".

# filter out the "git status" output (keeping the "On branch" line)
mv "$1" "$1".tmp
grep -v "^# [^O]" < "$1".tmp > "$1"
rm "$1".tmp

# append the diff
case "$1" in
*.stgit-edit.txt)
	stg show
;;
*)
	git diff --cached
;;
esac | sed -e 's/^/# /' >> "$1"

exec ${VISUAL:-${EDITOR:-vi}} "$1"
-- snap --

Hth,
Dscho

Re: [PATCH] Added giteditor script to show diff while editing commit message.

[Ted Pavlic]

Thanks for your comments. I've responded below. I just want to 
top-respond to your comment that the fundamental problem is that the 
diff is in a separate file. In fact, this is the point of the script. I 
want to be able to scroll through the diff output independent of the 
commit message.

(alternatively, I realize I could do "git commit -v" and then use my 
editor's "split window" support, but that wouldn't help me with "stg edit")

> the subject could use some work.  For example, I would prefix it with
...
>> From: Ted Pavlic<ted@tedpavlic.com>
>
> As this is exactly what your email said in its header, it is redundant
> information.  Worse, it is information that made me look back to know why
> it needs to be there.  Distracting.

I add --from to my gitsend alias to prevent git send-email from 
prompting me for a "From". Is there a way to have git send-email simply 
not prompt me for "From"?

> What information does the README add that is not in the script itself?
> If there is none, please refrain from adding the README to begin with.

OK. I noticed plenty of other not-very-useful READMEs in contrib/, and 
so I figured it was a pro forma file.

>> +# Find git
>> +[ -z "${GIT}" ]&&  GIT="git"
> Yes, I know it is contrib/, but you may want to adopt Git's coding style
> early.

Ok. Switching to test.

> Besides, I find it funny that you want to override git with $GIT.

Isn't it possible that someone has git somewhere else?

>> +# If we recognize a popular editor, add necessary flags
>> +case "${EDITOR}" in
>> +    emacs)
>> +        EDITOR="${EDITOR} -nw"
>
> Mhm.  Should this not be the user's choice?  Some like emacs to start up
> in a window.

I don't use emacs, but it was my impression that the "no window" flag 
was added to make sure that emacs doesn't fork. That's why "-f" is used 
in the vim line.

>> +# End GITTMP in ".git" so that "*.git/" syntax highlighting recognition
>> +# doesn't break
>> +GITTMP="${TMPDIR-/tmp}/giteditor.$RANDOM.$RANDOM.$RANDOM.$$.git"
>> +(umask 077&&  mkdir "${GITTMP}") || {
>> +    echo "Could not create temporary directory! Exiting." 1>&2
>> +    exit 1
>> +}
>
> Umm.  Why?  Why do you need a temporary .git directory?

The script generates a new "diff" file that I would rather drop 
elsewhere (e.g., in a /tmp directory) rather than here in the current 
directory.

However, maybe you're right. After all, stg drops ".stgit-edit.txt" in 
the working directory. I suppose I could use gitdir, but I wasn't sure 
if it was safe to pollute gitdir.

In the next version, I'll get rid of the temp directory and put the file 
here.

>> +    # Diff is non-empty, so edit msg and diff
>> +    ${EDITOR} "${GITTMP}/${COMMITMSG}" "${GITTMP}/diff" || exit $?
>
> vi users will hate you, as you do not give them a chance to edit the
> message after having seen the diff.

I don't see what you mean. I am a vi user (exclusively), and this script 
works very well for me.

The "-f -o" flags above ensure that gvim will not fork. I'll add "vi" to 
the search string that automatically add "-f -o". Will that satisfy you?

At the moment, giteditor works exactly like EDITOR (or VISUAL) for me, 
but it opens up a second buffer (split in the bottom window in my case) 
with the diff in it. I'm given the opportunity to save.

> git commit will abort anyway if the commit message has not changed.  Plus,
> it does a better job, as it checks only the non-commented-out text.

Okay. Using $1 exclusively.

> BTW why on earth do you put every single variable name in curly brackets?

I always thought that was good practice. It prevents ambiguity, and *I* 
don't think it's an eyesore.

> Besides all that criticism, there is also a fundamental issue.  The diff
> is in a separate file.

That's the point. If I wanted to put the diff in the commit buffer, I 
would have used "git commit -v". I think many would like to be able to 
scroll through the diff without having to scroll through the commit.

Is there no value in having the diff in a separate file?

Thanks --
Ted

-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

[PATCH] contrib: A script to show diff in new window while editing commit message.

[Ted Pavlic]

This new script (contrib/giteditor/giteditor) is an example GIT_EDITOR
that causes the editor to open the commit message as well as a "git diff
--cached" in a separate window. This behavior differs from "git commit
-v" in that the diff can be browsed independently of the commit message
without having to invoke a split window view in an editor.

This script also detects when "stg edit" is being called and uses "stg
show" instead. Hence, it implements a kind of "stg show -v".

This script is highly influenced by the "hgeditor" script distributed
with the Mercurial SCM.

It could be improved by supporting a command-line flag that would mimic
the "git commit -v"-type behavior of opening the diff in the same window
as the commit message. This would extend existing commands like "stg
edit" that do not already have a "-v"-type option.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
---

This version attempts to answer the concerns brought up by Johannes
Schindlin.

 contrib/giteditor/giteditor |   68 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)
 create mode 100755 contrib/giteditor/giteditor

diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
new file mode 100755
index 0000000..501a11c
--- /dev/null
+++ b/contrib/giteditor/giteditor
@@ -0,0 +1,68 @@
+#!/bin/sh
+#
+# Set GIT_EDITOR (or core.editor) to this script to see a diff alongside
+# commit message. This script differs from "git commit -v" in that the
+# diff shows up in a separate buffer. Additionally, this script works
+# with "stg edit" as well.
+#
+# Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
+# Highly influenced by hgeditor script distributed with Mercurial SCM.
+# Distributed under the GNU General Public License, version 2.0.
+
+# Find git
+test -z "${GIT}" && GIT="git"
+
+# Find stg
+test -z "${STG}" && STG="stg"
+
+# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
+EDITOR=${GIT_EDITOR_EDITOR-${VISUAL-${EDITOR-vi}}}
+
+# If we recognize a popular editor, add necessary flags (e.g., to
+# prevent forking)
+case "${EDITOR}" in
+    emacs)
+        EDITOR="${EDITOR} -nw"
+        ;;
+    mvim|gvim|vim|vi)
+        EDITOR="${EDITOR} -f -o"
+        ;;
+esac
+
+# Remove temporary files even if we get interrupted
+DIFFOUTPUT="giteditor.${RANDOM}.${RANDOM}.${RANDOM}.$$.diff"
+cleanup_exit() { 
+    rm -f "${DIFFOUTPUT}" 
+}
+trap "cleanup_exit" 0       # normal exit
+trap "exit 255" 1 2 3 6 15  # HUP INT QUIT ABRT TERM
+
+# For git, COMMITMSG=COMMIT_EDITMSG
+# For stg, COMMITMSG=.stgit-edit.txt
+# etc.
+COMMITMSG=$(basename "$1")
+case "${COMMITMSG}" in
+    .stgit-edit.txt)        # From "stg edit" 
+        DIFFCMD="${STG}"
+        DIFFARGS="show"
+        ;;
+    *)                      # Fall through to "git commit" case
+        DIFFCMD="${GIT}"
+        DIFFARGS="diff --cached"
+        # To focus on files that changed, use:
+        #DIFFARGS="diff --cached --diff-filter=M"
+        ;;
+esac
+
+"${DIFFCMD}" ${DIFFARGS} > ${DIFFOUTPUT}
+
+if test -s "${DIFFOUTPUT}"; then
+    # Diff is non-empty, so edit msg and diff
+    ${EDITOR} "$1" "${DIFFOUTPUT}" || exit $?
+else
+    # Empty diff. Only edit msg
+    ${EDITOR} "$1" || exit $?
+fi
+
+# (recall that DIFFOUTPUT file gets cleaned up by trap above)
+exit
-- 
1.6.1.213.g28da8

Re: [PATCH] Added giteditor script to show diff while editing commit message.

[Johannes Schindelin]

Hi,

On Wed, 21 Jan 2009, Ted Pavlic wrote:

> Thanks for your comments. I've responded below. I just want to 
> top-respond to your comment that the fundamental problem is that the 
> diff is in a separate file. In fact, this is the point of the script. I 
> want to be able to scroll through the diff output independent of the 
> commit message.

Once again, note that e.g. vi will not cope with the way you try to 
achieve that.

> > > From: Ted Pavlic<ted@tedpavlic.com>
> >
> > From: me
> 
> > Besides, I find it funny that you want to override git with $GIT.
> 
> Isn't it possible that someone has git somewhere else?

The script is called from within Git.  So you can rest assured that "git" 
is in the PATH, I guess.  Except in configurations where you have a 
properly installed GIT_EXEC_PATH, and run Git using an absolute path.

If you want to cater for such a case, sure go ahead :-)

> > > +# doesn't break
> > > +GITTMP="${TMPDIR-/tmp}/giteditor.$RANDOM.$RANDOM.$RANDOM.$$.git"
> > > +(umask 077&&  mkdir "${GITTMP}") || {
> > > +    echo "Could not create temporary directory! Exiting." 1>&2
> > > +    exit 1
> > > +}
> >
> > Umm.  Why?  Why do you need a temporary .git directory?
> 
> The script generates a new "diff" file that I would rather drop elsewhere
> (e.g., in a /tmp directory) rather than here in the current directory.

Why not .git/?  That would be the _natural_ place to put it.

> > > +    # Diff is non-empty, so edit msg and diff
> > > +    ${EDITOR} "${GITTMP}/${COMMITMSG}" "${GITTMP}/diff" || exit $?
> >
> > vi users will hate you, as you do not give them a chance to edit the
> > message after having seen the diff.
> 
> I don't see what you mean. I am a vi user (exclusively), and this script 
> works very well for me.

I cannot go back to the commit message when I said ":n" to get to the 
diff.

> > Besides all that criticism, there is also a fundamental issue.  The 
> > diff is in a separate file.
> 
> That's the point. If I wanted to put the diff in the commit buffer, I 
> would have used "git commit -v". I think many would like to be able to 
> scroll through the diff without having to scroll through the commit.
> 
> Is there no value in having the diff in a separate file?

In my case, no, for 2 reasons:

- I can always open a new shell (in ssh connections, I use screen) to get 
  the diff, and even better: I can restrict it to certain files, and I can 
  use the nice bookmarks "less" provides; dunno if vi would have them.

- My preference is definitely to look at the diff before committing, to be 
  certain that I did not fsck up.  And nothing would annoy me more than to 
  be in the middle of editing a commit message while I am looking at the 
  diff and telling myself "that is a stupid mistake, let's fix it" knowing 
  that the commit will not pick up the fix.

  So seeing the diff while composing the commit message is definitely too 
  late for me.

Ciao,
Dscho

Re: [PATCH] contrib: A script to show diff in new window while editing commit message.

[Junio C Hamano]

Ted Pavlic <ted@tedpavlic.com> writes:

> It could be improved by supporting a command-line flag that would mimic
> the "git commit -v"-type behavior of opening the diff in the same window
> as the commit message. This would extend existing commands like "stg
> edit" that do not already have a "-v"-type option.

If a single-buffer operation is an improvement, then I do not see the
point of this script.

 * Some people would like two-buffer operation and they may use this
   script as their core.editor.

 * Other people (including me) would find it very natural to use "\C-x 2"
   if they need to look at two places of the same buffer, because that is
   what they are used to do when editing a long file every day.  They just
   use "commit -v" without bothering with this script.

 * Yet other people (like Dscho) would find it too late to have a chance
   for final review when writing a commit log message anyway, and won't
   use either.

And I think choice is good.

Having said that, if the lack of "final chance to review the diff" in some
StGIT subcommand is the real problem you are trying to solve, I think it
is better solved by fixing StGIT.  If this script can be used as a
substitute for the real solution, that may be a welcome unintended side
effect, but I do not think you should make it the main selling point of
the script.  After all people may not want to use this script when they
are working directly with git, but still would want StGIT fixed.

Re: [PATCH] Added giteditor script to show diff while editing commit message.

[Ted Pavlic]

>> Thanks for your comments. I've responded below. I just want to
>> top-respond to your comment that the fundamental problem is that the
>> diff is in a separate file. In fact, this is the point of the script. I
>> want to be able to scroll through the diff output independent of the
>> commit message.
> Once again, note that e.g. vi will not cope with the way you try to
> achieve that.

It does for me just fine. In vi, I hit ^W^W and move from commit message 
to diff and back. What's the problem with that? In gvim I'm able to 
click back and forth.

I have been using this method for a long time with hg, and now I've been 
using it with git. This isn't theory --- it's been working in practice 
for me. Am I missing something?

> Why not .git/?  That would be the _natural_ place to put it.

Why doesn't stg do that? I figured stg would be a well-established 
program to pattern behavior off of.

I'll rev-parse the git dir and place the file there.

>>> vi users will hate you, as you do not give them a chance to edit the
>>> message after having seen the diff.
>> I don't see what you mean. I am a vi user (exclusively), and this script
>> works very well for me.
> I cannot go back to the commit message when I said ":n" to get to the
> diff.

vi opens for me and I see two windows. The top window shows the commit 
message and the bottom window shows the diff.

I hit ^W^W (or ^W<Down>) and I find myself scrolling around in the diff. 
I hit ^W^W again (or ^W<Up>) and I find myself scrolling around in the 
commit.

Similarly, gvim lets me mouse around both --- clicking from window to 
window.

If you must use ":n", I don't know why you can't use ":prev" to go back. <?>

>> Is there no value in having the diff in a separate file?
> In my case, no, for 2 reasons:
> - I can always open a new shell (in ssh connections, I use screen) to get
>    the diff, and even better: I can restrict it to certain files, and I can
>    use the nice bookmarks "less" provides; dunno if vi would have them.

vi does.

> - My preference is definitely to look at the diff before committing, to be
>    certain that I did not fsck up.  And nothing would annoy me more than to
>    be in the middle of editing a commit message while I am looking at the
>    diff and telling myself "that is a stupid mistake, let's fix it" knowing
>    that the commit will not pick up the fix.

When giving a detailed message bulletting out everything that goes into 
a commit, sometimes it's nice to have a very nearby look at the diff.

>    So seeing the diff while composing the commit message is definitely too
>    late for me.

Nevertheless, the secondary purpose of the contributed script is to show 
how GIT_EDITOR can be used to wrap around other editors. (that's the 
purpose of Mercurial's distributed "hgeditor" script as well)

--Ted


-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

Re: [PATCH] contrib: A script to show diff in new window while editing commit message.

[Ted Pavlic]

>   * Some people would like two-buffer operation and they may use this
>     script as their core.editor.

That's what I do, and evidently that's what enough Mercurial users do to 
warrant them putting "hgeditor" in their main distro (not even in the 
"contrib" directory). I just figured there must be at least a few other 
people out there like me, but maybe I'm wrong. <?>

>   * Other people (including me) would find it very natural to use "\C-x 2"
>     if they need to look at two places of the same buffer, because that is
>     what they are used to do when editing a long file every day.  They just
>     use "commit -v" without bothering with this script.

As I mention in the script, splitting windows (which is easy to do in 
Vim) is another very natural way to go. That being said, not all editors 
have terrific split window support. Plus, if you're going to do it 
often, it makes sense to wire up your editor to do it for you each time.

>   * Yet other people (like Dscho) would find it too late to have a chance
>     for final review when writing a commit log message anyway, and won't
>     use either.

I'm not using the diff as a code review. I'm using it to help me make 
sure my commit message is complete.

> Having said that, if the lack of "final chance to review the diff" in some
> StGIT subcommand is the real problem you are trying to solve, I think it
...

Having the "stg edit" support is just a bonus (for me). The main point 
was showing how to use GIT_EDITOR to bring up a split window. As with 
Mercurial's hgeditor, part of the point of the script is to demonstrate 
how to customize the commit process.

Thanks --
--Ted

-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

[PATCH] contrib: A script to show diff in new window while editing commit message.

[Ted Pavlic]

This new script (contrib/giteditor/giteditor) is an example GIT_EDITOR
that causes the editor to open the commit message as well as a "git diff
--cached" in a separate window. This behavior differs from "git commit
-v" in that the diff can be browsed independently of the commit message
without having to invoke a split window view in an editor.

This script also detects when "stg edit" is being called and uses "stg
show" instead. Hence, it implements a kind of "stg edit -v".

This script is highly influenced by the "hgeditor" script distributed
with the Mercurial SCM.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
---

This new commit responds to some of the issues brought up by Junio C
Hemano (and Johannes Schindelin). In particular, it removes the
paragraph from the commit message discussing how it could be "improved." 

Also, this new version uses a "DIFFPIPE" to strip the old commit message
from the top of the "stg show" output so that the "stg edit" behavior
matches the "git commit" behavior. 

Finally, this version adds a comment giving an idea for personalizing by
adding the temporary directory creation back in (as a way to prevent
editor backup files from piling up inside .git).

 contrib/giteditor/giteditor |   86 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 86 insertions(+), 0 deletions(-)
 create mode 100755 contrib/giteditor/giteditor

diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
new file mode 100755
index 0000000..5369732
--- /dev/null
+++ b/contrib/giteditor/giteditor
@@ -0,0 +1,86 @@
+#!/bin/sh
+#
+# Set GIT_EDITOR (or core.editor) to this script to see a diff alongside
+# commit message. This script differs from "git commit -v" in that the
+# diff shows up in a separate buffer. Additionally, this script works
+# with "stg edit" as well.
+#
+# Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
+# Highly influenced by hgeditor script distributed with Mercurial SCM.
+# Distributed under the GNU General Public License, version 2.0.
+#
+# Possible personalizations:
+#
+# * If your GIT_DIR gets polluted with backup files created by your
+#   editor when COMMIT_EDITMSG is saved, then have this script copy
+#   COMMIT_EDITMSG (i.e., $1) to a temporary directory and then back to
+#   COMMIT_EDITMSG when done. When the script cleans up after itself, it
+#   can delete the temporary directory and any leftover backup files.
+#   Note that the vim setting 'nobackup' disables saving backup files,
+#   and this setting can be set automatically on gitcommit-type files
+#   and files matching .stgit-*.txt with an appropriate ftdetect entry.
+
+# Find git
+test -z "${GIT}" && GIT="git"
+
+# Find stg
+test -z "${STG}" && STG="stg"
+
+# Find the nearest git-dir
+GITDIR=$(git rev-parse --git-dir) || exit
+
+# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
+EDITOR="${GIT_EDITOR_EDITOR-${VISUAL-${EDITOR-vi}}}"
+
+# If we recognize a popular editor, add necessary flags (e.g., to
+# prevent forking)
+case "${EDITOR}" in
+    emacs)
+        EDITOR="${EDITOR} -nw"
+        ;;
+    mvim|gvim|vim|vi)
+        EDITOR="${EDITOR} -f -o"
+        ;;
+esac
+
+# Remove temporary files even if we get interrupted
+DIFFOUTPUT="${GITDIR}/giteditor.${RANDOM}.${RANDOM}.${RANDOM}.$$.diff"
+cleanup_exit() { 
+    rm -f "${DIFFOUTPUT}" 
+}
+trap "cleanup_exit" 0       # normal exit
+trap "exit 255" 1 2 3 6 15  # HUP INT QUIT ABRT TERM
+
+# For git, COMMITMSG=COMMIT_EDITMSG
+# For stg, COMMITMSG=.stgit-edit.txt
+# etc.
+COMMITMSG=$(basename "$1")
+DIFFPIPE="cat"
+case "${COMMITMSG}" in
+    .stgit-edit.txt)        # From "stg edit" 
+        DIFFCMD="${STG}"
+        DIFFARGS="show"
+        DIFFPIPE="tail +$(wc -l "$1"|awk '{print $1+3}')"
+        ;;
+    *)                      # Fall through to "git commit" case
+        DIFFCMD="${GIT}"
+        DIFFARGS="diff --cached"
+        # To focus on files that changed, use:
+        #DIFFARGS="diff --cached --diff-filter=M"
+        ;;
+esac
+
+# Do the diff and save the result in DIFFOUTPUT
+"${DIFFCMD}" ${DIFFARGS} | ${DIFFPIPE} > ${DIFFOUTPUT}
+
+# If DIFFOUTPUT is nonempty, open it alongside commit message
+if test -s "${DIFFOUTPUT}"; then
+    # Diff is non-empty, so edit msg and diff
+    ${EDITOR} "$1" "${DIFFOUTPUT}" || exit
+else
+    # Empty diff. Only edit msg
+    ${EDITOR} "$1" || exit
+fi
+
+# (recall that DIFFOUTPUT file gets cleaned up by trap above)
+exit
-- 
1.6.1.213.g28da8

Re: [PATCH] contrib: A script to show diff in new window while editing commit message.

[Junio C Hamano]

Ted Pavlic <ted@tedpavlic.com> writes:

> This new script (contrib/giteditor/giteditor) is an example GIT_EDITOR
> that causes the editor to open the commit message as well as a "git diff
> --cached" in a separate window. This behavior differs from "git commit
> -v" in that the diff can be browsed independently of the commit message
> without having to invoke a split window view in an editor.
>
> This script also detects when "stg edit" is being called and uses "stg
> show" instead. Hence, it implements a kind of "stg edit -v".
>
> This script is highly influenced by the "hgeditor" script distributed
> with the Mercurial SCM.
>
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> ---
>
> This new commit responds to some of the issues brought up by Junio C
> Hemano (and Johannes Schindelin). In particular, it removes the
> paragraph from the commit message discussing how it could be "improved." 
>
> Also, this new version uses a "DIFFPIPE" to strip the old commit message
> from the top of the "stg show" output so that the "stg edit" behavior
> matches the "git commit" behavior. 
>
> Finally, this version adds a comment giving an idea for personalizing by
> adding the temporary directory creation back in (as a way to prevent
> editor backup files from piling up inside .git).
>
>  contrib/giteditor/giteditor |   86 +++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 86 insertions(+), 0 deletions(-)
>  create mode 100755 contrib/giteditor/giteditor
>
> diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
> new file mode 100755
> index 0000000..5369732
> --- /dev/null
> +++ b/contrib/giteditor/giteditor
> @@ -0,0 +1,86 @@
> +#!/bin/sh
> +#
> +# Set GIT_EDITOR (or core.editor) to this script to see a diff alongside
> +# commit message. This script differs from "git commit -v" in that the
> +# diff shows up in a separate buffer. Additionally, this script works
> +# with "stg edit" as well.
> +#
> +# Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
> +# Highly influenced by hgeditor script distributed with Mercurial SCM.
> +# Distributed under the GNU General Public License, version 2.0.
> +#
> +# Possible personalizations:
> +#
> +# * If your GIT_DIR gets polluted with backup files created by your
> +#   editor when COMMIT_EDITMSG is saved, then have this script copy
> +#   COMMIT_EDITMSG (i.e., $1) to a temporary directory and then back to
> +#   COMMIT_EDITMSG when done. When the script cleans up after itself, it
> +#   can delete the temporary directory and any leftover backup files.
> +#   Note that the vim setting 'nobackup' disables saving backup files,
> +#   and this setting can be set automatically on gitcommit-type files
> +#   and files matching .stgit-*.txt with an appropriate ftdetect entry.

I am not sure what problem you are trying to offer a solution here.  Are
you suggesting a trick to avoid .git/COMMIT_EDITMSG~ left behind by Emacs?

> +# Find git
> +test -z "${GIT}" && GIT="git"

Why?

> +# Find stg
> +test -z "${STG}" && STG="stg"

Why?

> +# Find the nearest git-dir
> +GITDIR=$(git rev-parse --git-dir) || exit
> +
> +# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
> +EDITOR="${GIT_EDITOR_EDITOR-${VISUAL-${EDITOR-vi}}}"

At the beginning of this file, you have a nice insn to set GIT_EDITOR, but
this GIT_EDITOR_EDITOR to customize what underlying editor you end up
launching should also be documented in the same place.

I do not think you need the dq pair around the right hand side.

> +# If we recognize a popular editor, add necessary flags (e.g., to
> +# prevent forking)
> +case "${EDITOR}" in
> +    emacs)
> +        EDITOR="${EDITOR} -nw"
> +        ;;
> +    mvim|gvim|vim|vi)
> +        EDITOR="${EDITOR} -f -o"
> +        ;;
> +esac

 - Please align case arm labels with case and esac, like this (I do not
   mind 4-space indentation if that is what you are used to):

	case "$foo" in
        bar)
        	... do bar things ...
                ;;
	...
	esac

 - Braces around variables are noisy and distracting;

 - Why force emacs users to -nw?  If _you_ personally like -nw, shouldn't
   you be able to simply do:

	GIT_EDITOR_EDITOR="emacs -nw"

   in your environment?  After all, when you use $EDITOR later, you do not
   quote it and let $IFS separate the flags the variable may have in
   addition to the name of (or path to) the executable.

> +# Remove temporary files even if we get interrupted
> +DIFFOUTPUT="${GITDIR}/giteditor.${RANDOM}.${RANDOM}.${RANDOM}.$$.diff"

The script begins with #!/bin/sh but isn't ${RANDOM} a Bash-ism?  Either
you should begin it with #!/bin/bash, or avoid bash-ism, if you do not
want to alienate poeple whose /bin/sh is not bash.

> +cleanup_exit() { 
> +    rm -f "${DIFFOUTPUT}" 
> +}
> +trap "cleanup_exit" 0       # normal exit
> +trap "exit 255" 1 2 3 6 15  # HUP INT QUIT ABRT TERM

It may have been useful while debugging this script, but is the temporary
file that precious to it needs to be kept upon HUP and friends?

> +# For git, COMMITMSG=COMMIT_EDITMSG
> +# For stg, COMMITMSG=.stgit-edit.txt
> +# etc.
> +COMMITMSG=$(basename "$1")
> +DIFFPIPE="cat"
> +case "${COMMITMSG}" in
> +    .stgit-edit.txt)        # From "stg edit" 
> +        DIFFCMD="${STG}"
> +        DIFFARGS="show"
> +        DIFFPIPE="tail +$(wc -l "$1"|awk '{print $1+3}')"
> +        ;;
> +    *)                      # Fall through to "git commit" case
> +        DIFFCMD="${GIT}"
> +        DIFFARGS="diff --cached"
> +        # To focus on files that changed, use:
> +        #DIFFARGS="diff --cached --diff-filter=M"
> +        ;;
> +esac
> +

This '*' case is horribly wrong.

What happens if other parts of git (or third party tools around git) runs
core.editor for things other than the commit log messages (or anything you
do not know how to prepare a sensible diff to show)?  Shouldn't you act as
if you were not there at all to avoid surprising the end user with an
unexpected diff output?

For example, doesn't "git rebase -i" launch core.editor to let you edit
the pick/edit/squash insn sequence?  What diff are you showing in such a
case, and how would that help your users?

> +# Do the diff and save the result in DIFFOUTPUT
> +"${DIFFCMD}" ${DIFFARGS} | ${DIFFPIPE} > ${DIFFOUTPUT}

Because your "extra information in another file" processing depends
heavily on what the edited file is (you are already generating the diff
differently for "git commit" and "stg edit"), I think having this outside
the above case statement is a false factoring; it would be easier to read
and maintain if they are defined in each case arm.  I would probably write
this part like this:

	case "$COMMITMSG" in
        .stgit-edit.txt)
        	do whatever appropriate for "stg edit"  and emit to stdout
                ;;
	COMMIT_EDITMSG)
        	if git rev-parse -q --verify HEAD >/dev/null
                then
                	git diff --cached
		else
                        git diff --cached 4b825dc642cb6eb9a060e54bf8d69288fbee4904
                fi
                ;;
	*)
        	: we do not know how to handle this one
                ;;
	esac >"$DIFFOUTPUT"

You need to quote "$DIFFOUTPUT"; it is a path in $GIT_DIR which means it
can have $IFS character.

> +# If DIFFOUTPUT is nonempty, open it alongside commit message
> +if test -s "${DIFFOUTPUT}"; then
> +    # Diff is non-empty, so edit msg and diff
> +    ${EDITOR} "$1" "${DIFFOUTPUT}" || exit
> +else
> +    # Empty diff. Only edit msg
> +    ${EDITOR} "$1" || exit

In the latter case, it may be cleaner to:

	rm -f "$DIFFOUTPUT"
        exec ${EDITOR} "$1"

One more thing.  You may want to study how git_editor() in git-sh-setup
scriptlet solves the issue of (1) the path to the editor executable may
have $IFS character that the end user may want to quote, and (2) the end
user may want to include options to the editor in the varaible, by using
eval.

> +fi
> +
> +# (recall that DIFFOUTPUT file gets cleaned up by trap above)
> +exit