Merge branch 'jh/notes-add-ui'

* jh/notes-add-ui:
  Make "git notes add" more user-friendly when there are existing notes

Conflicts:
	builtin/notes.c

3 files changed, 49 insertions, 7 deletions

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 296f314eae..913ecd8c43 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -57,8 +57,11 @@ list::
 
 add::
 	Add notes for a given object (defaults to HEAD). Abort if the
-	object already has notes (use `-f` to overwrite an
-	existing note).
+	object already has notes (use `-f` to overwrite existing notes).
+	However, if you're using `add` interactively (using an editor
+	to supply the notes contents), then - instead of aborting -
+	the existing notes will be opened in the editor (like the `edit`
+	subcommand).
 
 copy::
 	Copy the notes for the first object onto the second object.
diff --git a/builtin/notes.c b/builtin/notes.c
index d6dcfcb014..8685d2bda6 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -529,6 +529,8 @@ static int list(int argc, const char **argv, const char *prefix)
 	return retval;
 }
 
+static int append_edit(int argc, const char **argv, const char *prefix);
+
 static int add(int argc, const char **argv, const char *prefix)
 {
 	int retval = 0, force = 0;
@@ -556,14 +558,14 @@ static int add(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options, git_notes_add_usage,
-			     0);
+			     PARSE_OPT_KEEP_ARGV0);
 
-	if (1 < argc) {
+	if (2 < argc) {
 		error(_("too many parameters"));
 		usage_with_options(git_notes_add_usage, options);
 	}
 
-	object_ref = argc ? argv[0] : "HEAD";
+	object_ref = argc > 1 ? argv[1] : "HEAD";
 
 	if (get_sha1(object_ref, object))
 		die(_("Failed to resolve '%s' as a valid ref."), object_ref);
@@ -573,6 +575,18 @@ static int add(int argc, const char **argv, const char *prefix)
 
 	if (note) {
 		if (!force) {
+			if (!msg.given) {
+				/*
+				 * Redirect to "edit" subcommand.
+				 *
+				 * We only end up here if none of -m/-F/-c/-C
+				 * or -f are given. The original args are
+				 * therefore still in argv[0-1].
+				 */
+				argv[0] = "edit";
+				free_notes(t);
+				return append_edit(argc, argv, prefix);
+			}
 			retval = error(_("Cannot add notes. Found existing notes "
 				       "for object %s. Use '-f' to overwrite "
 				       "existing notes"), sha1_to_hex(object));
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 1921ca3a73..3448c23776 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -101,8 +101,8 @@ test_expect_success 'edit existing notes' '
 	test_must_fail git notes show HEAD^
 '
 
-test_expect_success 'cannot add note where one exists' '
-	! MSG=b2 git notes add &&
+test_expect_success 'cannot "git notes add -m" where notes already exists' '
+	test_must_fail git notes add -m "b2" &&
 	test ! -f .git/NOTES_EDITMSG &&
 	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
 	test b3 = $(git notes show) &&
@@ -110,6 +110,24 @@ test_expect_success 'cannot add note where one exists' '
 	test_must_fail git notes show HEAD^
 '
 
+test_expect_success 'can overwrite existing note with "git notes add -f -m"' '
+	git notes add -f -m "b1" &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b1 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
+test_expect_success 'add w/no options on existing note morphs into edit' '
+	MSG=b2 git notes add &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b2 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
 test_expect_success 'can overwrite existing note with "git notes add -f"' '
 	MSG=b1 git notes add -f &&
 	test ! -f .git/NOTES_EDITMSG &&
@@ -194,6 +212,13 @@ test_expect_success 'show -F notes' '
 	test_cmp expect-F output
 '
 
+test_expect_success 'Re-adding -F notes without -f fails' '
+	echo "zyxxy" > note5 &&
+	test_must_fail git notes add -F note5 &&
+	git log -3 > output &&
+	test_cmp expect-F output
+'
+
 cat >expect << EOF
 commit 15023535574ded8b1a89052b32673f84cf9582b8
 tree e070e3af51011e47b183c33adf9736736a525709