Make "git notes add" more user-friendly when there are existing notes
Currently, "notes add" (without -f/--force) will abort when the given object already has existing notes. This makes sense for the modes of "git notes add" that would necessarily overwrite the old message (when using the -m/-F/-C/-c options). However, when no options are given (meaning the notes are created from scratch in the editor) it is not very user-friendly to abort on existing notes, and forcing the user to run "git notes edit". Instead, it is better to simply "redirect" to "git notes edit" automatically, i.e. open the existing notes in the editor and let the user edit them. This patch does just that. This changes the behavior of "git notes add" without options when notes already exist for the given object, but I doubt that many users really depend on the previous failure from "git notes add" in this case. Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johan Herland committed
Mar 30, 2011 at 02:02 UTC
84a7e35eea97388ba5ca458808119650818c2fd2
3 files changed
+49
-7
Documentation/git-notes.txt
+5
-2
@@ -57,8 +57,11 @@ list::
57
58
add::
59
Add notes for a given object (defaults to HEAD). Abort if the
60
- object already has notes (use `-f` to overwrite an
61
- existing note).
60
+ object already has notes (use `-f` to overwrite existing notes).
61
+ However, if you're using `add` interactively (using an editor
62
+ to supply the notes contents), then - instead of aborting -
63
+ the existing notes will be opened in the editor (like the `edit`
64
+ subcommand).
65
66
copy::
67
Copy the notes for the first object onto the second object.
builtin/notes.c
+17
-3
@@ -527,6 +527,8 @@ static int list(int argc, const char **argv, const char *prefix)
527
return retval;
528
}
529
530
+static int append_edit(int argc, const char **argv, const char *prefix);
531
+
532
static int add(int argc, const char **argv, const char *prefix)
533
{
534
int retval = 0, force = 0;
@@ -554,14 +556,14 @@ static int add(int argc, const char **argv, const char *prefix)
556
};
557
558
argc = parse_options(argc, argv, prefix, options, git_notes_add_usage,
557
- 0);
559
+ PARSE_OPT_KEEP_ARGV0);
560
559
- if (1 < argc) {
561
+ if (2 < argc) {
562
error("too many parameters");
563
usage_with_options(git_notes_add_usage, options);
564
}
565
564
- object_ref = argc ? argv[0] : "HEAD";
566
+ object_ref = argc > 1 ? argv[1] : "HEAD";
567
568
if (get_sha1(object_ref, object))
569
die("Failed to resolve '%s' as a valid ref.", object_ref);
@@ -571,6 +573,18 @@ static int add(int argc, const char **argv, const char *prefix)
573
574
if (note) {
575
if (!force) {
576
+ if (!msg.given) {
577
+ /*
578
+ * Redirect to "edit" subcommand.
579
+ *
580
+ * We only end up here if none of -m/-F/-c/-C
581
+ * or -f are given. The original args are
582
+ * therefore still in argv[0-1].
583
+ */
584
+ argv[0] = "edit";
585
+ free_notes(t);
586
+ return append_edit(argc, argv, prefix);
587
+ }
588
retval = error("Cannot add notes. Found existing notes "
589
"for object %s. Use '-f' to overwrite "
590
"existing notes", sha1_to_hex(object));
t/t3301-notes.sh
+27
-2
@@ -101,8 +101,8 @@ test_expect_success 'edit existing notes' '
101
test_must_fail git notes show HEAD^
102
'
103
104
-test_expect_success 'cannot add note where one exists' '
105
- ! MSG=b2 git notes add &&
104
+test_expect_success 'cannot "git notes add -m" where notes already exists' '
105
+ test_must_fail git notes add -m "b2" &&
106
test ! -f .git/NOTES_EDITMSG &&
107
test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
108
test b3 = $(git notes show) &&
@@ -110,6 +110,24 @@ test_expect_success 'cannot add note where one exists' '
110
test_must_fail git notes show HEAD^
111
'
112
113
+test_expect_success 'can overwrite existing note with "git notes add -f -m"' '
114
+ git notes add -f -m "b1" &&
115
+ test ! -f .git/NOTES_EDITMSG &&
116
+ test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
117
+ test b1 = $(git notes show) &&
118
+ git show HEAD^ &&
119
+ test_must_fail git notes show HEAD^
120
+'
121
+
122
+test_expect_success 'add w/no options on existing note morphs into edit' '
123
+ MSG=b2 git notes add &&
124
+ test ! -f .git/NOTES_EDITMSG &&
125
+ test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
126
+ test b2 = $(git notes show) &&
127
+ git show HEAD^ &&
128
+ test_must_fail git notes show HEAD^
129
+'
130
+
131
test_expect_success 'can overwrite existing note with "git notes add -f"' '
132
MSG=b1 git notes add -f &&
133
test ! -f .git/NOTES_EDITMSG &&
@@ -194,6 +212,13 @@ test_expect_success 'show -F notes' '
212
test_cmp expect-F output
213
'
214
215
+test_expect_success 'Re-adding -F notes without -f fails' '
216
+ echo "zyxxy" > note5 &&
217
+ test_must_fail git notes add -F note5 &&
218
+ git log -3 > output &&
219
+ test_cmp expect-F output
220
+'
221
+
222
cat >expect << EOF
223
commit 15023535574ded8b1a89052b32673f84cf9582b8
224
tree e070e3af51011e47b183c33adf9736736a525709