notes.c: introduce '--separator=<paragraph-break>' option

When adding new notes or appending to an existing notes, we will insert a blank line between the paragraphs, like: $ git notes add -m foo -m bar $ git notes show HEAD foo bar The default behavour sometimes is not enough, the user may want to use a custom delimiter between paragraphs, like when specifying '-m', '-F', '-C', '-c' options. So this commit introduce a new '--separator' option for 'git notes add' and 'git notes append', for example when executing: $ git notes add -m foo -m bar --separator="-" $ git notes show HEAD foo - bar a newline is added to the value given to --separator if it does not end with one already. So when executing: $ git notes add -m foo -m bar --separator="-" and $ export LF=" " $ git notes add -m foo -m bar --separator="-$LF" Both the two exections produce the same result. The reason we use a "strbuf" array to concat but not "string_list", is that the binary file content may contain '\0' in the middle, this will cause the corrupt result if using a string to save. Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Teng Long committed May 27, 2023 at 15:57 UTC 90bc19b3aede4ebe8b1044541373ac970804cde4
3 files changed +227 -29
Documentation/git-notes.txt
+16 -5
@@ -9,9 +9,9 @@ SYNOPSIS
9 --------
10 [verse]
11 'git notes' [list [<object>]]
12 -'git notes' add [-f] [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
12 +'git notes' add [-f] [--allow-empty] [--separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
13 'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )
14 -'git notes' append [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
14 +'git notes' append [--allow-empty] [--separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
15 'git notes' edit [--allow-empty] [<object>]
16 'git notes' show [<object>]
17 'git notes' merge [-v | -q] [-s <strategy> ] <notes-ref>
@@ -65,7 +65,9 @@ add::
65 However, if you're using `add` interactively (using an editor
66 to supply the notes contents), then - instead of aborting -
67 the existing notes will be opened in the editor (like the `edit`
68 - subcommand).
68 + subcommand). If you specify multiple `-m` and `-F`, a blank
69 + line will be inserted between the messages. Use the `--separator`
70 + option to insert other delimiters.
71
72 copy::
73 Copy the notes for the first object onto the second object (defaults to
@@ -85,8 +87,12 @@ corresponding <to-object>. (The optional `<rest>` is ignored so that
87 the command can read the input given to the `post-rewrite` hook.)
88
89 append::
88 - Append to the notes of an existing object (defaults to HEAD).
89 - Creates a new notes object if needed.
90 + Append new message(s) given by `-m` or `-F` options to an
91 + existing note, or add them as a new note if one does not
92 + exist, for the object (defaults to HEAD). When appending to
93 + an existing note, a blank line is added before each new
94 + message as an inter-paragraph separator. The separator can
95 + be customized with the `--separator` option.
96
97 edit::
98 Edit the notes for a given object (defaults to HEAD).
@@ -159,6 +165,11 @@ OPTIONS
165 Allow an empty note object to be stored. The default behavior is
166 to automatically remove empty notes.
167
168 +--separator <paragraph-break>::
169 + Specify a string used as a custom inter-paragraph separator
170 + (a newline is added at the end as needed). Defaults to a
171 + blank line.
172 +
173 --ref <ref>::
174 Manipulate the notes tree in <ref>. This overrides
175 `GIT_NOTES_REF` and the "core.notesRef" configuration. The ref
builtin/notes.c
+85 -24
@@ -8,6 +8,7 @@
8 */
9
10 #include "cache.h"
11 +#include "alloc.h"
12 #include "config.h"
13 #include "builtin.h"
14 #include "gettext.h"
@@ -27,11 +28,12 @@
28 #include "worktree.h"
29 #include "write-or-die.h"
30
31 +static char *separator = "\n";
32 static const char * const git_notes_usage[] = {
33 N_("git notes [--ref <notes-ref>] [list [<object>]]"),
32 - N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
34 + N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
35 N_("git notes [--ref <notes-ref>] copy [-f] <from-object> <to-object>"),
34 - N_("git notes [--ref <notes-ref>] append [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
36 + N_("git notes [--ref <notes-ref>] append [--allow-empty] [--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
37 N_("git notes [--ref <notes-ref>] edit [--allow-empty] [<object>]"),
38 N_("git notes [--ref <notes-ref>] show [<object>]"),
39 N_("git notes [--ref <notes-ref>] merge [-v | -q] [-s <strategy>] <notes-ref>"),
@@ -99,11 +101,19 @@ static const char * const git_notes_get_ref_usage[] = {
101 static const char note_template[] =
102 N_("Write/edit the notes for the following object:");
103
104 +struct note_msg {
105 + int stripspace;
106 + struct strbuf buf;
107 +};
108 +
109 struct note_data {
110 int given;
111 int use_editor;
112 char *edit_path;
113 struct strbuf buf;
114 + struct note_msg **messages;
115 + size_t msg_nr;
116 + size_t msg_alloc;
117 };
118
119 static void free_note_data(struct note_data *d)
@@ -113,6 +123,12 @@ static void free_note_data(struct note_data *d)
123 free(d->edit_path);
124 }
125 strbuf_release(&d->buf);
126 +
127 + while (d->msg_nr--) {
128 + strbuf_release(&d->messages[d->msg_nr]->buf);
129 + free(d->messages[d->msg_nr]);
130 + }
131 + free(d->messages);
132 }
133
134 static int list_each_note(const struct object_id *object_oid,
@@ -213,65 +229,97 @@ static void write_note_data(struct note_data *d, struct object_id *oid)
229 }
230 }
231
232 +static void insert_separator(struct strbuf *message, size_t pos)
233 +{
234 + size_t sep_len = strlen(separator);
235 + if (sep_len && separator[sep_len - 1] == '\n')
236 + strbuf_insertstr(message, pos, separator);
237 + else
238 + strbuf_insertf(message, pos, "%s%s", separator, "\n");
239 +}
240 +
241 +static void concat_messages(struct note_data *d)
242 +{
243 + struct strbuf msg = STRBUF_INIT;
244 +
245 + size_t i;
246 + for (i = 0; i < d->msg_nr ; i++) {
247 + if (d->buf.len)
248 + insert_separator(&d->buf, d->buf.len);
249 + strbuf_add(&msg, d->messages[i]->buf.buf, d->messages[i]->buf.len);
250 + strbuf_addbuf(&d->buf, &msg);
251 + if (d->messages[i]->stripspace)
252 + strbuf_stripspace(&d->buf, 0);
253 + strbuf_reset(&msg);
254 + }
255 + strbuf_release(&msg);
256 +}
257 +
258 static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
259 {
260 struct note_data *d = opt->value;
261 + struct note_msg *msg = xmalloc(sizeof(*msg));
262
263 BUG_ON_OPT_NEG(unset);
264
222 - if (d->buf.len)
223 - strbuf_addch(&d->buf, '\n');
224 - strbuf_addstr(&d->buf, arg);
225 - strbuf_stripspace(&d->buf, 0);
226 -
227 - d->given = 1;
265 + strbuf_init(&msg->buf, strlen(arg));
266 + strbuf_addstr(&msg->buf, arg);
267 + ALLOC_GROW_BY(d->messages, d->msg_nr, 1, d->msg_alloc);
268 + d->messages[d->msg_nr - 1] = msg;
269 + msg->stripspace = 1;
270 return 0;
271 }
272
273 static int parse_file_arg(const struct option *opt, const char *arg, int unset)
274 {
275 struct note_data *d = opt->value;
276 + struct note_msg *msg = xmalloc(sizeof(*msg));
277
278 BUG_ON_OPT_NEG(unset);
279
237 - if (d->buf.len)
238 - strbuf_addch(&d->buf, '\n');
280 + strbuf_init(&msg->buf , 0);
281 if (!strcmp(arg, "-")) {
240 - if (strbuf_read(&d->buf, 0, 1024) < 0)
282 + if (strbuf_read(&msg->buf, 0, 1024) < 0)
283 die_errno(_("cannot read '%s'"), arg);
242 - } else if (strbuf_read_file(&d->buf, arg, 1024) < 0)
284 + } else if (strbuf_read_file(&msg->buf, arg, 1024) < 0)
285 die_errno(_("could not open or read '%s'"), arg);
244 - strbuf_stripspace(&d->buf, 0);
286
246 - d->given = 1;
287 + ALLOC_GROW_BY(d->messages, d->msg_nr, 1, d->msg_alloc);
288 + d->messages[d->msg_nr - 1] = msg;
289 + msg->stripspace = 1;
290 return 0;
291 }
292
293 static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
294 {
295 struct note_data *d = opt->value;
253 - char *buf;
296 + struct note_msg *msg = xmalloc(sizeof(*msg));
297 + char *value;
298 struct object_id object;
299 enum object_type type;
300 unsigned long len;
301
302 BUG_ON_OPT_NEG(unset);
303
260 - if (d->buf.len)
261 - strbuf_addch(&d->buf, '\n');
262 -
304 + strbuf_init(&msg->buf, 0);
305 if (repo_get_oid(the_repository, arg, &object))
306 die(_("failed to resolve '%s' as a valid ref."), arg);
265 - if (!(buf = repo_read_object_file(the_repository, &object, &type, &len)))
307 + if (!(value = repo_read_object_file(the_repository, &object, &type, &len)))
308 die(_("failed to read object '%s'."), arg);
309 if (type != OBJ_BLOB) {
268 - free(buf);
310 + strbuf_release(&msg->buf);
311 + free(value);
312 + free(msg);
313 die(_("cannot read note data from non-blob object '%s'."), arg);
314 }
271 - strbuf_add(&d->buf, buf, len);
272 - free(buf);
315
274 - d->given = 1;
316 + strbuf_add(&msg->buf, value, len);
317 + free(value);
318 +
319 + msg->buf.len = len;
320 + ALLOC_GROW_BY(d->messages, d->msg_nr, 1, d->msg_alloc);
321 + d->messages[d->msg_nr - 1] = msg;
322 + msg->stripspace = 0;
323 return 0;
324 }
325
@@ -406,6 +454,7 @@ static int add(int argc, const char **argv, const char *prefix)
454 struct object_id object, new_note;
455 const struct object_id *note;
456 struct note_data d = { .buf = STRBUF_INIT };
457 +
458 struct option options[] = {
459 OPT_CALLBACK_F('m', "message", &d, N_("message"),
460 N_("note contents as a string"), PARSE_OPT_NONEG,
@@ -422,6 +471,8 @@ static int add(int argc, const char **argv, const char *prefix)
471 OPT_BOOL(0, "allow-empty", &allow_empty,
472 N_("allow storing empty note")),
473 OPT__FORCE(&force, N_("replace existing notes"), PARSE_OPT_NOCOMPLETE),
474 + OPT_STRING(0, "separator", &separator, N_("separator"),
475 + N_("insert <paragraph-break> between paragraphs")),
476 OPT_END()
477 };
478
@@ -433,6 +484,10 @@ static int add(int argc, const char **argv, const char *prefix)
484 usage_with_options(git_notes_add_usage, options);
485 }
486
487 + if (d.msg_nr)
488 + concat_messages(&d);
489 + d.given = !!d.buf.len;
490 +
491 object_ref = argc > 1 ? argv[1] : "HEAD";
492
493 if (repo_get_oid(the_repository, object_ref, &object))
@@ -587,6 +642,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
642 parse_reuse_arg),
643 OPT_BOOL(0, "allow-empty", &allow_empty,
644 N_("allow storing empty note")),
645 + OPT_STRING(0, "separator", &separator, N_("separator"),
646 + N_("insert <paragraph-break> between paragraphs")),
647 OPT_END()
648 };
649 int edit = !strcmp(argv[0], "edit");
@@ -600,6 +657,10 @@ static int append_edit(int argc, const char **argv, const char *prefix)
657 usage_with_options(usage, options);
658 }
659
660 + if (d.msg_nr)
661 + concat_messages(&d);
662 + d.given = !!d.buf.len;
663 +
664 if (d.given && edit)
665 fprintf(stderr, _("The -m/-F/-c/-C options have been deprecated "
666 "for the 'edit' subcommand.\n"
@@ -623,7 +684,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
684 &type, &size);
685
686 if (d.buf.len && prev_buf && size)
626 - strbuf_insertstr(&d.buf, 0, "\n");
687 + insert_separator(&d.buf, 0);
688 if (prev_buf && size)
689 strbuf_insert(&d.buf, 0, prev_buf, size);
690 free(prev_buf);
t/t3301-notes.sh
+126
@@ -362,6 +362,7 @@ test_expect_success 'do not create empty note with -m ""' '
362 '
363
364 test_expect_success 'create note with combination of -m and -F' '
365 + test_when_finished git notes remove HEAD &&
366 cat >expect-combine_m_and_F <<-EOF &&
367 foo
368
@@ -380,6 +381,25 @@ test_expect_success 'create note with combination of -m and -F' '
381 test_cmp expect-combine_m_and_F actual
382 '
383
384 +test_expect_success 'create note with combination of -m and -F and --separator' '
385 + cat >expect-combine_m_and_F <<-\EOF &&
386 + foo
387 + -------
388 + xyzzy
389 + -------
390 + bar
391 + -------
392 + zyxxy
393 + -------
394 + baz
395 + EOF
396 + echo "xyzzy" >note_a &&
397 + echo "zyxxy" >note_b &&
398 + git notes add -m "foo" -F note_a -m "bar" -F note_b -m "baz" --separator "-------" &&
399 + git notes show >actual &&
400 + test_cmp expect-combine_m_and_F actual
401 +'
402 +
403 test_expect_success 'remove note with "git notes remove"' '
404 git notes remove HEAD^ &&
405 git notes remove &&
@@ -521,6 +541,85 @@ test_expect_success 'listing non-existing notes fails' '
541 test_must_be_empty actual
542 '
543
544 +test_expect_success 'append: specify an empty separator' '
545 + test_when_finished git notes remove HEAD &&
546 + cat >expect <<-\EOF &&
547 + notes-1
548 +
549 + notes-2
550 + EOF
551 +
552 + git notes add -m "notes-1" &&
553 + git notes append --separator="" -m "notes-2" &&
554 + git notes show >actual &&
555 + test_cmp expect actual
556 +'
557 +
558 +test_expect_success 'append: specify separator with line break' '
559 + test_when_finished git notes remove HEAD &&
560 + cat >expect <<-\EOF &&
561 + notes-1
562 + -------
563 + notes-2
564 + EOF
565 +
566 + git notes add -m "notes-1" &&
567 + git notes append --separator="-------$LF" -m "notes-2" &&
568 + git notes show >actual &&
569 + test_cmp expect actual
570 +'
571 +
572 +test_expect_success 'append: specify separator without line break' '
573 + test_when_finished git notes remove HEAD &&
574 + cat >expect <<-\EOF &&
575 + notes-1
576 + -------
577 + notes-2
578 + EOF
579 +
580 + git notes add -m "notes-1" &&
581 + git notes append --separator="-------" -m "notes-2" &&
582 + git notes show >actual &&
583 + test_cmp expect actual
584 +'
585 +
586 +test_expect_success 'append: specify separator with multiple messages' '
587 + test_when_finished git notes remove HEAD &&
588 + cat >expect <<-\EOF &&
589 + notes-1
590 + -------
591 + notes-2
592 + -------
593 + notes-3
594 + EOF
595 +
596 + git notes add -m "notes-1" &&
597 + git notes append --separator="-------" -m "notes-2" -m "notes-3" &&
598 + git notes show >actual &&
599 + test_cmp expect actual
600 +'
601 +
602 +test_expect_success 'append note with combination of -m and -F and --separator' '
603 + test_when_finished git notes remove HEAD &&
604 + cat >expect-combine_m_and_F <<-\EOF &&
605 + m-notes-1
606 + -------
607 + f-notes-1
608 + -------
609 + m-notes-2
610 + -------
611 + f-notes-2
612 + -------
613 + m-notes-3
614 + EOF
615 +
616 + echo "f-notes-1" >note_a &&
617 + echo "f-notes-2" >note_b &&
618 + git notes append -m "m-notes-1" -F note_a -m "m-notes-2" -F note_b -m "m-notes-3" --separator "-------" &&
619 + git notes show >actual &&
620 + test_cmp expect-combine_m_and_F actual
621 +'
622 +
623 test_expect_success 'append to existing note with "git notes append"' '
624 cat >expect <<-EOF &&
625 Initial set of notes
@@ -818,6 +917,33 @@ test_expect_success 'create note from blob with "git notes add -C" reuses blob i
917 test_cmp blob actual
918 '
919
920 +test_expect_success 'create note from blob with "-C", also specify "-m", "-F" and "--separator"' '
921 + # 8th will be reuseed in following tests, so rollback when the test is done
922 + test_when_finished "git notes remove && git notes add -C $(cat blob)" &&
923 + commit=$(git rev-parse HEAD) &&
924 + cat >expect <<-EOF &&
925 + commit $commit
926 + Author: A U Thor <author@example.com>
927 + Date: Thu Apr 7 15:20:13 2005 -0700
928 +
929 + ${indent}8th
930 +
931 + Notes:
932 + ${indent}This is a blob object
933 + ${indent}-------
934 + ${indent}This is created by -m
935 + ${indent}-------
936 + ${indent}This is created by -F
937 + EOF
938 +
939 + git notes remove &&
940 + echo "This is a blob object" | git hash-object -w --stdin >blob &&
941 + echo "This is created by -F" >note_a &&
942 + git notes add -C $(cat blob) -m "This is created by -m" -F note_a --separator="-------" &&
943 + git log -1 >actual &&
944 + test_cmp expect actual
945 +'
946 +
947 test_expect_success 'create note from other note with "git notes add -c"' '
948 test_commit 9th &&
949 commit=$(git rev-parse HEAD) &&