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

From: Ted Pavlic <ted@tedpavlic.com>
Date: Jan 21, 2009, 22:33 UTC
Message-ID: <4977A2C9.1070502@tedpavlic.com>
In-Reply-To: <alpine.DEB.1.00.0901212216310.3586@pacific.mpi-cbg.de>

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

...

Show 5 quoted lines

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

Show 7 quoted lines

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

Show 9 quoted lines

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

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

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.