Subject: Re: [PATCH Cogito] Make use of external editor work like CVS

From: Marcel Holtmann <[email-masked]>
Date: Sun, 08 May 2005 17:43:10 +0200

Hi Petr,

> > this is a modified version of my patch that integrates the your latest
> > modifications to cg-commit and also fixes the cleanup of the temporary
> > files when we abort the operation.
> 
> ...and I've just pushed more modifications. I'm so evil.

attached is another version of the patch.

> > [PATCH] Make use of external editor work like CVS
> > 
> > The lines starting with `CG:' should be a trailer and not at the top
> > of the message presented in the editor. Also extend the number of `-'
> > up to 74 characters so that people know when they should start a new
> > line. If it's not a merge and no commit text is given as parameter
> > then add an extra empty line at the top. And don't forget to take
> > care of the temporary files when a commit is unneeded or canceled.
> > 
> > Signed-off-by: Marcel Holtmann <[email-masked]>
> 
> What is so special about 74 columns? Why not 75 (fmt default), or 72
> (emails)?

I ended up with 74, because "CG" has only two letters instead of "CVS"
which has three. And cg-log uses a prefix of four whitespaces. This
leaves two free characters at the end of a line if your terminal uses a
width of 80 characters. The decision was of cosmetic nature.

> Also, I'd prefer the empty line to be always there in front of the CG:
> stuff (two empty lines in case of merge - I want to encourage people to
> keep possible details w.r.t. the merge separated by an empty line from
> the merge information), and when reading it back cg-commit should strip
> any trailing empty lines.

I think we should differentiate between the merges. There is no need for
additional information if it is an automatic merge (no conflicts) and in
general it makes no sense to open the editor (until forced). I wanted to
address this later. And yes in case of a manual merge it is a good idea
to add two extra empty lines at the top.

Another good idea is maybe to remove empty lines at the top and bottom
before doing the commit.

> > Index: cg-commit
> > ===================================================================
> > --- 8bb38f8bfdc7411460c300c811da1987173f412f/cg-commit  (mode:100755)
> > +++ be440e169fa3b5ec5450fa9574cd8789b0e3ab20/cg-commit  (mode:100755)
> >  if [ "$merging" ]; then
> > -	echo -n 'Merge with ' >>$LOGMSG
> > -	[ "$msgs" ] && echo -n 'Merge with '
> > +	echo -n "Merge with " >>$LOGMSG
> > +	[ "$msgs" ] && echo -n "Merge with "
> 
> We aren't too consistent about this anyway now, so you might as well
> let it not clutter your patch. ;-)

I was moving parts of the code so I addressed it ;)

> >  cp $LOGMSG $LOGMSG2
> >  if tty -s; then
> >  	if ! [ "$msgs" ]; then
> > -		${EDITOR:-vi} $LOGMSG2
> > -		[ $LOGMSG2 -nt $LOGMSG ] || die 'Commit message not modified, commit aborted'
> > +		${EDITOR:-vi} $LOGMSG
> > +		if [ ! $LOGMSG -nt $LOGMSG2 ]; then
> > +			rm $LOGMSG $LOGMSG2
> > +			die 'Commit message not modified, commit aborted.'
> > +		fi
> >  	fi
> >  else
> > -	cat >>$LOGMSG2
> > +	cat >>$LOGMSG
> >  fi
> > -grep -v ^CG: $LOGMSG2 >$LOGMSG
> > -rm $LOGMSG2
> > +grep -v ^CG: $LOGMSG >$LOGMSG2
> > +mv $LOGMSG2 $LOGMSG
> 
> Why are you messing with the $LOGMSG variables here?

This is only cosmetic. Using vim it displays the name of the temporary
file and confusing the user with gitci2.XXXX instead of gitci.XXX is
weird. Even using gitci as basename looks not good to me, but I left it
for now.

Regards

Marcel

[PATCH] Make use of external editor work like CVS

The lines starting with `CG:' should be a trailer and not at the top
of the message presented in the editor. Also extend the number of `-'
up to 74 characters so that people know when they should start a new
line. If it's not a merge and no commit text is given as parameter
then add an extra empty line at the top. And don't forget to take
care of the temporary files when a commit is unneeded or canceled.

Signed-off-by: Marcel Holtmann <[email-masked]>

---
commit a2c4e793a4dfb21f43494ad90c7b887df10e1be2
tree cb61efa8a01400150162af9b0f3773f21d502fe9
parent 79f86b0174159f016540734ac18560566389b823
author Marcel Holtmann <[email-masked]> Sun, 08 May 2005 17:30:01 +0200
committer Marcel Holtmann <[email-masked]> Sun, 08 May 2005 17:30:01 +0200

 cg-commit |   61 +++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 35 insertions(+), 26 deletions(-)

Index: cg-commit
===================================================================
--- f00d7589973e8ea65d2264f5fbac82e1b217dc8f/cg-commit  (mode:100755)
+++ cb61efa8a01400150162af9b0f3773f21d502fe9/cg-commit  (mode:100755)
@@ -67,26 +67,10 @@
 
 LOGMSG=$(mktemp -t gitci.XXXXXX)
 LOGMSG2=$(mktemp -t gitci2.XXXXXX)
-echo CG: ---------------------------------------------------------- >>$LOGMSG
-echo CG: Lines beggining with CG: will be automatically removed     >>$LOGMSG
-echo CG:                                                            >>$LOGMSG
-if [ ! "$ignorecache" ]; then
-	if [ ! "${commitfiles[*]}" ]; then
-		echo 'Nothing to commit.' >&2
-		exit 2
-	fi
-	for file in "${commitfiles[@]}"; do
-		# Prepend a letter describing whether it's addition,
-		# removal or update. Or call git status on those files.
-		echo CG: $file >>$LOGMSG
-		[ "$msgs" ] && echo $file
-	done
-	echo CG: >>$LOGMSG
-fi
 
 if [ "$merging" ]; then
-	echo -n 'Merge with ' >>$LOGMSG
-	[ "$msgs" ] && echo -n 'Merge with '
+	echo -n "Merge with " >>$LOGMSG
+	[ "$msgs" ] && echo -n "Merge with "
 	[ -s .git/merging-sym ] || cp .git/merging .git/merging-sym
 	for sym in $(cat .git/merging-sym); do
 		uri=$(cat .git/branches/$sym)
@@ -94,30 +78,55 @@
 		echo "$uri" >>$LOGMSG
 		[ "$msgs" ] && echo "$uri"
 	done
-	echo >>$LOGMSG
+else
+	first=1
 fi
-first=1
+
 for msg in "${msgs[@]}"; do
 	if [ "$first" ]; then
 		first=
 	else
 		echo >>$LOGMSG
 	fi
-	echo $msg | fmt >>$LOGMSG
+	echo $msg | fmt -s -w 74 >>$LOGMSG
 done
+
+if [ "$first" ]; then
+	echo >>$LOGMSG
+fi
+
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+echo "CG: Enter Log.  Lines beginning with \`CG:' are removed automatically"      >>$LOGMSG
+if [ ! "$ignorecache" ]; then
+	if [ ! "${commitfiles[*]}" ]; then
+		rm $LOGMSG $LOGMSG2
+		die 'Nothing to commit.'
+	fi
+	echo "CG: " >>$LOGMSG
+	echo "CG: Modified Files:" >>$LOGMSG
+	for file in "${commitfiles[@]}"; do
+		# Prepend a letter describing whether it's addition,
+		# removal or update. Or call git status on those files.
+		echo "CG:    $file" >>$LOGMSG
+		[ "$msgs" ] && echo $file
+	done
+fi
+echo "CG: ----------------------------------------------------------------------" >>$LOGMSG
+
 cp $LOGMSG $LOGMSG2
 if tty -s; then
 	if ! [ "$msgs" ] || [ "$forceeditor" ]; then
-		${EDITOR:-vi} $LOGMSG2
+		${EDITOR:-vi} $LOGMSG
 	fi
-	if ! [ "$msgs" ] && ! [ $LOGMSG2 -nt $LOGMSG ]; then
-		die 'Commit message not modified, commit aborted'
+	if ! [ "$msgs" ] && ! [ $LOGMSG -nt $LOGMSG2 ]; then
+		rm $LOGMSG $LOGMSG2
+		die 'Commit message not modified, commit aborted.'
 	fi
 else
 	cat >>$LOGMSG2
 fi
-grep -v ^CG: $LOGMSG2 >$LOGMSG
-rm $LOGMSG2
+grep -v ^CG: $LOGMSG >$LOGMSG2
+mv $LOGMSG2 $LOGMSG
 
 if [ ! "$ignorecache" ]; then
 	if [ "$customfiles" ]; then