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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 21, 2009, 22:52 UTC
Message-ID: <alpine.DEB.1.00.0901212343100.3586@pacific.mpi-cbg.de>
In-Reply-To: <4977A2C9.1070502@tedpavlic.com>

Hi,

On Wed, 21 Jan 2009, Ted Pavlic wrote:

Show 5 quoted lines

> 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.

Show 7 quoted lines

> > > 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 :-)

Show 11 quoted lines

> > > +# 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.

Show 8 quoted lines

> > > +    # 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.

Show 8 quoted lines

> > 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