[PATCH 0/2] Add "-m" and "-F" options to "git notes edit"

[PATCH 0/2] Add "-m" and "-F" options to "git notes edit"

[Johan Herland]

Hi,

The following 2-patch series teaches "git notes edit" to support the
"-m <msg>" and "-F <file>" options (inspired by git-tag and git-commit).

The first patch does some minor bugfixes and cleanups in preparation for
the second patch, which adds the options with associated documentation
and selftests.

This series is based on top of the "js/notes" topic in a recent "pu".

Johan Herland (2):
  Minor cleanup and bugfixing in git-notes.sh
  Teach "-m <msg>" and "-F <file>" to "git notes edit"

 Documentation/git-notes.txt |   12 +++++++-
 git-notes.sh                |   71 ++++++++++++++++++++++++++++++++----------
 t/t3301-notes.sh            |   57 ++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 18 deletions(-)


Have fun! :)

...Johan

[PATCH 1/2] Minor cleanup and bugfixing in git-notes.sh

[Johan Herland]

The following changes are made:
- Rename $MESSAGE to $MSG_FILE which better describes its purpose
- Make sure not only $MSG_FILE is removed upopn completion, but also
  $GIT_INDEX_FILE
- Remove stray "GIT_NOTES_REF=" in front of "git log ..." command

Signed-off-by: Johan Herland <johan@herland.net>
---
 git-notes.sh |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/git-notes.sh b/git-notes.sh
index 6ec33c9..7c3b8b9 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -20,15 +20,16 @@ edit)
 		die "Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)"
 	fi
 
-	MESSAGE="$GIT_DIR"/new-notes-$COMMIT
+	MSG_FILE="$GIT_DIR/new-notes-$COMMIT"
+	GIT_INDEX_FILE="MSG_FILE.idx"
+	export GIT_INDEX_FILE
+
 	trap '
-		test -f "$MESSAGE" && rm "$MESSAGE"
+		test -f "$MSG_FILE" && rm "$MSG_FILE"
+		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
 	' 0
 
-	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
-
-	GIT_INDEX_FILE="$MESSAGE".idx
-	export GIT_INDEX_FILE
+	git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
 
 	CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
 	if [ -z "$CURRENT_HEAD" ]; then
@@ -36,16 +37,16 @@ edit)
 	else
 		PARENT="-p $CURRENT_HEAD"
 		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
-		git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
+		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
 	fi
 
 	core_editor="$(git config core.editor)"
-	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MESSAGE"
+	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
 
-	grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
-	mv "$MESSAGE".processed "$MESSAGE"
-	if [ -s "$MESSAGE" ]; then
-		BLOB=$(git hash-object -w "$MESSAGE") ||
+	grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
+	mv "$MSG_FILE".processed "$MSG_FILE"
+	if [ -s "$MSG_FILE" ]; then
+		BLOB=$(git hash-object -w "$MSG_FILE") ||
 			die "Could not write into object database"
 		git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
 			die "Could not write index"
-- 
1.6.3.rc0.1.gf800

Re: [PATCH 1/2] Minor cleanup and bugfixing in git-notes.sh

[Johannes Schindelin]

Hi,

On Tue, 21 Apr 2009, Johan Herland wrote:

> The following changes are made:
> - Rename $MESSAGE to $MSG_FILE which better describes its purpose
> - Make sure not only $MSG_FILE is removed upopn completion, but also
>   $GIT_INDEX_FILE
> - Remove stray "GIT_NOTES_REF=" in front of "git log ..." command

Valid points.  Mind if I squash this into the commit adding git-notes.sh?

Ciao,
Dscho

Re: [PATCH 1/2] Minor cleanup and bugfixing in git-notes.sh

[Johan Herland]

On Tuesday 21 April 2009, Johannes Schindelin wrote:
> On Tue, 21 Apr 2009, Johan Herland wrote:
> > The following changes are made:
> > - Rename $MESSAGE to $MSG_FILE which better describes its purpose
> > - Make sure not only $MSG_FILE is removed upopn completion, but also
> >   $GIT_INDEX_FILE
> > - Remove stray "GIT_NOTES_REF=" in front of "git log ..." command
>
> Valid points.  Mind if I squash this into the commit adding git-notes.sh?

No problem


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

[PATCH 2/2] Teach "-m <msg>" and "-F <file>" to "git notes edit"

[Johan Herland]

The "-m" and "-F" options are already the established method
(in both git-commit and git-tag) to specify a commit/tag message
without invoking the editor. This patch teaches "git notes edit"
to respect the same options for specifying a notes message without
invoking the editor.

The patch also updates the "git notes" documentation and adds a
couple of selftests for the new functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |   12 ++++++++-
 git-notes.sh                |   52 +++++++++++++++++++++++++++++++++------
 t/t3301-notes.sh            |   57 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 7136016..55de7c0 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -8,7 +8,7 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git-notes' (edit | show) [commit]
+'git-notes' (edit [-F <file> | -m <msg>] | show) [commit]
 
 DESCRIPTION
 -----------
@@ -33,6 +33,16 @@ show::
 	Show the notes for a given commit (defaults to HEAD).
 
 
+OPTIONS
+-------
+-m <msg>::
+	Use the given note message (instead of prompting).
+
+-F <file>::
+	Take the note message from the given file.  Use '-' to
+	read the note message from the standard input.
+
+
 Author
 ------
 Written by Johannes Schindelin <johannes.schindelin@gmx.de>
diff --git a/git-notes.sh b/git-notes.sh
index 7c3b8b9..3911caa 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -1,16 +1,47 @@
 #!/bin/sh
 
-USAGE="(edit | show) [commit]"
+USAGE="(edit [-F <file> | -m <msg>] | show) [commit]"
 . git-sh-setup
 
-test -n "$3" && usage
-
 test -z "$1" && usage
 ACTION="$1"; shift
 
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
 
+MESSAGE=
+while test $# != 0
+do
+	case "$1" in
+	-m)
+		test "$ACTION" = "edit" || usage
+		shift
+		if test "$#" = "0"; then
+			die "error: option -m needs an argument"
+		else
+			MESSAGE="$1"
+			shift
+		fi
+		;;
+	-F)
+		test "$ACTION" = "edit" || usage
+		shift
+		if test "$#" = "0"; then
+			die "error: option -F needs an argument"
+		else
+			MESSAGE="$(cat "$1")"
+			shift
+		fi
+		;;
+	-*)
+		usage
+		;;
+	*)
+		break
+		;;
+	esac
+done
+
 COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
 die "Invalid commit: $@"
 
@@ -29,19 +60,24 @@ edit)
 		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
 	' 0
 
-	git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
-
 	CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
 	if [ -z "$CURRENT_HEAD" ]; then
 		PARENT=
 	else
 		PARENT="-p $CURRENT_HEAD"
 		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
-		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
 	fi
 
-	core_editor="$(git config core.editor)"
-	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+	if [ -n "$MESSAGE" ]; then
+		echo "$MESSAGE" > "$MSG_FILE"
+	else
+		git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
+		if [ -n "$CURRENT_HEAD" ]; then
+			git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
+		fi
+		core_editor="$(git config core.editor)"
+		${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+	fi
 
 	grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
 	mv "$MSG_FILE".processed "$MSG_FILE"
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 73e53be..2ece75e 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -110,5 +110,62 @@ test_expect_success 'show multi-line notes' '
 	git log -2 > output &&
 	test_cmp expect-multiline output
 '
+test_expect_success 'create -m notes (setup)' '
+	: > a4 &&
+	git add a4 &&
+	test_tick &&
+	git commit -m 4th &&
+	git notes edit -m "foo
+bar
+baz"
+'
+
+cat > expect-m << EOF
+commit 15023535574ded8b1a89052b32673f84cf9582b8
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:16:13 2005 -0700
+
+    4th
+
+Notes:
+    foo
+    bar
+    baz
+EOF
+
+printf "\n" >> expect-m
+cat expect-multiline >> expect-m
+
+test_expect_success 'show -m notes' '
+	git log -3 > output &&
+	test_cmp expect-m output
+'
+test_expect_success 'create -F notes (setup)' '
+	: > a5 &&
+	git add a5 &&
+	test_tick &&
+	git commit -m 5th &&
+	echo "xyzzy" > note5 &&
+	git notes edit -F note5
+'
+
+cat > expect-F << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:17:13 2005 -0700
+
+    5th
+
+Notes:
+    xyzzy
+EOF
+
+printf "\n" >> expect-F
+cat expect-m >> expect-F
+
+test_expect_success 'show -F notes' '
+	git log -4 > output &&
+	test_cmp expect-F output
+'
 
 test_done
-- 
1.6.3.rc0.1.gf800

Re: [PATCH 2/2] Teach "-m <msg>" and "-F <file>" to "git notes edit"

[Johannes Schindelin]

Hi,

On Tue, 21 Apr 2009, Johan Herland wrote:

> The "-m" and "-F" options are already the established method
> (in both git-commit and git-tag) to specify a commit/tag message
> without invoking the editor. This patch teaches "git notes edit"
> to respect the same options for specifying a notes message without
> invoking the editor.
> 
> The patch also updates the "git notes" documentation and adds a
> couple of selftests for the new functionality.
> 
> Signed-off-by: Johan Herland <johan@herland.net>
> ---

Nice!

> +MESSAGE=
> +while test $# != 0
> +do
> +	case "$1" in
> +	-m)
> +		test "$ACTION" = "edit" || usage
> +		shift
> +		if test "$#" = "0"; then
> +			die "error: option -m needs an argument"
> +		else
> +			MESSAGE="$1"
> +			shift
> +		fi
> +		;;
> +	-F)
> +		test "$ACTION" = "edit" || usage
> +		shift
> +		if test "$#" = "0"; then
> +			die "error: option -F needs an argument"
> +		else
> +			MESSAGE="$(cat "$1")"
> +			shift
> +		fi
> +		;;
> +	-*)
> +		usage
> +		;;
> +	*)
> +		break
> +		;;
> +	esac
> +done

Well, either you forbid multiple -F/-m options, or you merge the messages, 
right?  AFAIR 'git commit -m A -m B' combines 'A' and 'B'...

> +	if [ -n "$MESSAGE" ]; then

Here's a chance for me to learn: I was under the impression that "! -z" is 
more portable than "-n".  Am I completely off the track?

Ciao,
Dscho

[PATCH v2 2/2] Teach "-m <msg>" and "-F <file>" to "git notes edit"

[Johan Herland]

The "-m" and "-F" options are already the established method
(in both git-commit and git-tag) to specify a commit/tag message
without invoking the editor. This patch teaches "git notes edit"
to respect the same options for specifying a notes message without
invoking the editor.

Multiple "-m" and/or "-F" options are concatenated as separate
paragraphs.

The patch also updates the "git notes" documentation and adds
selftests for the new functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---

On Tuesday 21 April 2009, Johannes Schindelin wrote:
> Well, either you forbid multiple -F/-m options, or you merge the
> messages, right?  AFAIR 'git commit -m A -m B' combines 'A' and 'B'...

Here's a second version that merges the messages.

> > +	if [ -n "$MESSAGE" ]; then
> Here's a chance for me to learn: I was under the impression that "! -z"
> is more portable than "-n".  Am I completely off the track?

No, I haven't checked this at all, so you're probably right. This second
version of the patch does not use "-n".

Note that I changed the selftest to test the -m/-F merge behaviour.
This causes the expected output to have a couple of lines with 4 trailing
spaces. I know that trailing spaces is frowned upon on this list, but I'm
hoping these'll make it through, as the test needs them...


Have fun! :)

...Johan

 Documentation/git-notes.txt |   16 ++++++++++-
 git-notes.sh                |   64 +++++++++++++++++++++++++++++++++++++-----
 t/t3301-notes.sh            |   35 +++++++++++++++++++++++
 3 files changed, 106 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 7136016..94cceb1 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -8,7 +8,7 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git-notes' (edit | show) [commit]
+'git-notes' (edit [-F <file> | -m <msg>] | show) [commit]
 
 DESCRIPTION
 -----------
@@ -33,6 +33,20 @@ show::
 	Show the notes for a given commit (defaults to HEAD).
 
 
+OPTIONS
+-------
+-m <msg>::
+	Use the given note message (instead of prompting).
+	If multiple `-m` (or `-F`) options are given, their
+	values are concatenated as separate paragraphs.
+
+-F <file>::
+	Take the note message from the given file.  Use '-' to
+	read the note message from the standard input.
+	If multiple `-F` (or `-m`) options are given, their
+	values are concatenated as separate paragraphs.
+
+
 Author
 ------
 Written by Johannes Schindelin <johannes.schindelin@gmx.de>
diff --git a/git-notes.sh b/git-notes.sh
index 7c3b8b9..5ff82f2 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -1,16 +1,59 @@
 #!/bin/sh
 
-USAGE="(edit | show) [commit]"
+USAGE="(edit [-F <file> | -m <msg>] | show) [commit]"
 . git-sh-setup
 
-test -n "$3" && usage
-
 test -z "$1" && usage
 ACTION="$1"; shift
 
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
 
+MESSAGE=
+while test $# != 0
+do
+	case "$1" in
+	-m)
+		test "$ACTION" = "edit" || usage
+		shift
+		if test "$#" = "0"; then
+			die "error: option -m needs an argument"
+		else
+			if [ -z "$MESSAGE" ]; then
+				MESSAGE="$1"
+			else
+				MESSAGE="$MESSAGE
+
+$1"
+			fi
+			shift
+		fi
+		;;
+	-F)
+		test "$ACTION" = "edit" || usage
+		shift
+		if test "$#" = "0"; then
+			die "error: option -F needs an argument"
+		else
+			if [ -z "$MESSAGE" ]; then
+				MESSAGE="$(cat "$1")"
+			else
+				MESSAGE="$MESSAGE
+
+$(cat "$1")"
+			fi
+			shift
+		fi
+		;;
+	-*)
+		usage
+		;;
+	*)
+		break
+		;;
+	esac
+done
+
 COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
 die "Invalid commit: $@"
 
@@ -29,19 +72,24 @@ edit)
 		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
 	' 0
 
-	git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
-
 	CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
 	if [ -z "$CURRENT_HEAD" ]; then
 		PARENT=
 	else
 		PARENT="-p $CURRENT_HEAD"
 		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
-		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
 	fi
 
-	core_editor="$(git config core.editor)"
-	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+	if [ -z "$MESSAGE" ]; then
+		git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
+		if [ ! -z "$CURRENT_HEAD" ]; then
+			git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
+		fi
+		core_editor="$(git config core.editor)"
+		${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+	else
+		echo "$MESSAGE" > "$MSG_FILE"
+	fi
 
 	grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
 	mv "$MSG_FILE".processed "$MSG_FILE"
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 73e53be..9eaa338 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -110,5 +110,40 @@ test_expect_success 'show multi-line notes' '
 	git log -2 > output &&
 	test_cmp expect-multiline output
 '
+test_expect_success 'create -m and -F notes (setup)' '
+	: > a4 &&
+	git add a4 &&
+	test_tick &&
+	git commit -m 4th &&
+	echo "xyzzy" > note5 &&
+	git notes edit -m spam -F note5 -m "foo
+bar
+baz"
+'
+
+cat > expect-m-and-F << EOF
+commit 15023535574ded8b1a89052b32673f84cf9582b8
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:16:13 2005 -0700
+
+    4th
+
+Notes:
+    spam
+    
+    xyzzy
+    
+    foo
+    bar
+    baz
+EOF
+
+printf "\n" >> expect-m-and-F
+cat expect-multiline >> expect-m-and-F
+
+test_expect_success 'show -m and -F notes' '
+	git log -3 > output &&
+	test_cmp expect-m-and-F output
+'
 
 test_done
-- 
1.6.3.rc0.1.gf800

Re: [PATCH 0/2] Add "-m" and "-F" options to "git notes edit"

[Johannes Schindelin]

Hi,

On Tue, 21 Apr 2009, Johan Herland wrote:

> The following 2-patch series teaches "git notes edit" to support the
> "-m <msg>" and "-F <file>" options (inspired by git-tag and git-commit).

Good idea!

Ciao,
Dscho