From: Teng Long <dyroneteng@gmail.com>
Date: Sat, 27 May 2023 07:57:47 GMT
Subject: [PATCH v11 0/7] notes.c: introduce "--separator"
Message-ID: <cover.1685174011.git.dyroneteng@gmail.com>
In-Reply-To: <cover.1684411136.git.dyroneteng@gmail.com>

From: Teng Long <dyroneteng@gmail.com>

1.  "--[no-]separator" to an independent patch which suggested by Junio[1].
2.  fix "UBSan" problem which found by Peff[2].

[1] https://public-inbox.org/git/xmqqfs7tvfc7.fsf@gitster.g/
[2] https://public-inbox.org/git/20230519005447.GA2955320@coredump.intra.peff.net/

Thanks.

Teng Long (7):
  notes.c: cleanup 'strbuf_grow' call in 'append_edit'
  notes.c: use designated initializers for clarity
  t3321: add test cases about the notes stripspace behavior
  notes.c: introduce '--separator=<paragraph-break>' option
  notes.c: append separator instead of insert by pos
  notes.c: introduce "--[no-]stripspace" option
  notes: introduce "--no-separator" option

 Documentation/git-notes.txt |  43 ++-
 builtin/notes.c             | 159 +++++++---
 t/t3301-notes.sh            | 169 +++++++++++
 t/t3321-notes-stripspace.sh | 577 ++++++++++++++++++++++++++++++++++++
 4 files changed, 906 insertions(+), 42 deletions(-)
 create mode 100755 t/t3321-notes-stripspace.sh

Range-diff against v10:
1:  0634434e = 1:  0634434e notes.c: cleanup 'strbuf_grow' call in 'append_edit'
2:  4ad78405 = 2:  4ad78405 notes.c: use designated initializers for clarity
3:  c2fc2091 = 3:  c2fc2091 t3321: add test cases about the notes stripspace behavior
4:  820dda04 ! 4:  367cb1d4 notes.c: introduce '[--[no-]separator|--separator=<paragraph-break>]' option
    @@ Metadata
     Author: Teng Long <dyroneteng@gmail.com>
     
      ## Commit message ##
    -    notes.c: introduce '[--[no-]separator|--separator=<paragraph-break>]' option
    +    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:
    @@ Commit message
     
         Both the two exections produce the same result.
     
    -    Alternatively, if you do not want any new paragraph
    -    separators, even a newline by default, you can specify
    -    '--no-separator'.
    -
         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.
    @@ Documentation/git-notes.txt: SYNOPSIS
      [verse]
      'git notes' [list [<object>]]
     -'git notes' add [-f] [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    -+'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    ++'git notes' add [-f] [--allow-empty] [--separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
      'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )
     -'git notes' append [--allow-empty] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    -+'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    ++'git notes' append [--allow-empty] [--separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
      'git notes' edit [--allow-empty] [<object>]
      'git notes' show [<object>]
      'git notes' merge [-v | -q] [-s <strategy> ] <notes-ref>
    @@ Documentation/git-notes.txt: OPTIONS
      	Allow an empty note object to be stored. The default behavior is
      	to automatically remove empty notes.
      
    -+--[no-]separator, --separator=<paragraph-break>::
    ++--separator <paragraph-break>::
     +	Specify a string used as a custom inter-paragraph separator
    -+	(a newline is added at the end as needed). If `--no-separator`, no
    -+	separators will be added between paragraphs.  Defaults to a blank
    -+	line.
    ++	(a newline is added at the end as needed).  Defaults to a
    ++	blank line.
     +
      --ref <ref>::
      	Manipulate the notes tree in <ref>.  This overrides
    @@ builtin/notes.c
      #include "worktree.h"
      #include "write-or-die.h"
      
    -+static const char *separator = "\n";
    ++static char *separator = "\n";
      static const char * const git_notes_usage[] = {
      	N_("git notes [--ref <notes-ref>] [list [<object>]]"),
     -	N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    -+	N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    ++	N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
      	N_("git notes [--ref <notes-ref>] copy [-f] <from-object> <to-object>"),
     -	N_("git notes [--ref <notes-ref>] append [--allow-empty] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    -+	N_("git notes [--ref <notes-ref>] append [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    ++	N_("git notes [--ref <notes-ref>] append [--allow-empty] [--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
      	N_("git notes [--ref <notes-ref>] edit [--allow-empty] [<object>]"),
      	N_("git notes [--ref <notes-ref>] show [<object>]"),
      	N_("git notes [--ref <notes-ref>] merge [-v | -q] [-s <strategy>] <notes-ref>"),
    @@ builtin/notes.c: static void write_note_data(struct note_data *d, struct object_
      
     +static void insert_separator(struct strbuf *message, size_t pos)
     +{
    -+	if (!separator)
    -+		return;
    -+	else if (separator[strlen(separator) - 1] == '\n')
    ++	size_t sep_len = strlen(separator);
    ++	if (sep_len && separator[sep_len - 1] == '\n')
     +		strbuf_insertstr(message, pos, separator);
     +	else
     +		strbuf_insertf(message, pos, "%s%s", separator, "\n");
    @@ builtin/notes.c: static void write_note_data(struct note_data *d, struct object_
     +static void concat_messages(struct note_data *d)
     +{
     +	struct strbuf msg = STRBUF_INIT;
    -+	size_t i;
     +
    ++	size_t i;
     +	for (i = 0; i < d->msg_nr ; i++) {
     +		if (d->buf.len)
     +			insert_separator(&d->buf, d->buf.len);
    @@ builtin/notes.c: static void write_note_data(struct note_data *d, struct object_
      	return 0;
      }
      
    -@@ builtin/notes.c: static int parse_reedit_arg(const struct option *opt, const char *arg, int unset
    - 	return parse_reuse_arg(opt, arg, unset);
    - }
    - 
    -+static int parse_separator_arg(const struct option *opt, const char *arg,
    -+			       int unset)
    -+{
    -+	if (unset)
    -+		*(const char **)opt->value = NULL;
    -+	else
    -+		*(const char **)opt->value = arg ? arg : "\n";
    -+	return 0;
    -+}
    -+
    - static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
    - {
    - 	struct strbuf buf = STRBUF_INIT;
     @@ builtin/notes.c: static int add(int argc, const char **argv, const char *prefix)
      	struct object_id object, new_note;
      	const struct object_id *note;
    @@ builtin/notes.c: static int add(int argc, const char **argv, const char *prefix)
      		OPT_BOOL(0, "allow-empty", &allow_empty,
      			N_("allow storing empty note")),
      		OPT__FORCE(&force, N_("replace existing notes"), PARSE_OPT_NOCOMPLETE),
    -+		OPT_CALLBACK_F(0, "separator", &separator,
    -+			N_("<paragraph-break>"),
    -+			N_("insert <paragraph-break> between paragraphs"),
    -+			PARSE_OPT_OPTARG, parse_separator_arg),
    ++		OPT_STRING(0, "separator", &separator, N_("separator"),
    ++			N_("insert <paragraph-break> between paragraphs")),
      		OPT_END()
      	};
      
    @@ builtin/notes.c: static int append_edit(int argc, const char **argv, const char
      			parse_reuse_arg),
      		OPT_BOOL(0, "allow-empty", &allow_empty,
      			N_("allow storing empty note")),
    -+		OPT_CALLBACK_F(0, "separator", &separator,
    -+			N_("<paragraph-break>"),
    -+			N_("insert <paragraph-break> between paragraphs"),
    -+			PARSE_OPT_OPTARG, parse_separator_arg),
    ++		OPT_STRING(0, "separator", &separator, N_("separator"),
    ++			N_("insert <paragraph-break> between paragraphs")),
      		OPT_END()
      	};
      	int edit = !strcmp(argv[0], "edit");
    @@ t/t3301-notes.sh: test_expect_success 'create note with combination of -m and -F
      '
      
     +test_expect_success 'create note with combination of -m and -F and --separator' '
    -+	test_when_finished git notes remove HEAD &&
     +	cat >expect-combine_m_and_F <<-\EOF &&
     +	foo
     +	-------
    @@ t/t3301-notes.sh: test_expect_success 'create note with combination of -m and -F
     +	EOF
     +	echo "xyzzy" >note_a &&
     +	echo "zyxxy" >note_b &&
    -+	git notes add -m "foo" -F note_a -m "bar" -F note_b -m "baz" --separator="-------" &&
    -+	git notes show >actual &&
    -+	test_cmp expect-combine_m_and_F actual
    -+'
    -+
    -+test_expect_success 'create note with combination of -m and -F and --no-separator' '
    -+	cat >expect-combine_m_and_F <<-\EOF &&
    -+	foo
    -+	xyzzy
    -+	bar
    -+	zyxxy
    -+	baz
    -+	EOF
    -+	echo "xyzzy" >note_a &&
    -+	echo "zyxxy" >note_b &&
    -+	git notes add -m "foo" -F note_a -m "bar" -F note_b -m "baz" --no-separator &&
    ++	git notes add -m "foo" -F note_a -m "bar" -F note_b -m "baz" --separator "-------" &&
     +	git notes show >actual &&
     +	test_cmp expect-combine_m_and_F actual
     +'
    @@ t/t3301-notes.sh: test_expect_success 'listing non-existing notes fails' '
      	test_must_be_empty actual
      '
      
    -+test_expect_success 'append: specify a separator with an empty arg' '
    ++test_expect_success 'append: specify an empty separator' '
     +	test_when_finished git notes remove HEAD &&
     +	cat >expect <<-\EOF &&
     +	notes-1
    @@ t/t3301-notes.sh: test_expect_success 'listing non-existing notes fails' '
     +	test_cmp expect actual
     +'
     +
    -+test_expect_success 'append: specify a separator without arg' '
    -+	test_when_finished git notes remove HEAD &&
    -+	cat >expect <<-\EOF &&
    -+	notes-1
    -+
    -+	notes-2
    -+	EOF
    -+
    -+	git notes add -m "notes-1" &&
    -+	git notes append --separator -m "notes-2" &&
    -+	git notes show >actual &&
    -+	test_cmp expect actual
    -+'
    -+
    -+test_expect_success 'append: specify as --no-separator' '
    -+	test_when_finished git notes remove HEAD &&
    -+	cat >expect <<-\EOF &&
    -+	notes-1
    -+	notes-2
    -+	EOF
    -+
    -+	git notes add -m "notes-1" &&
    -+	git notes append --no-separator -m "notes-2" &&
    -+	git notes show >actual &&
    -+	test_cmp expect actual
    -+'
    -+
     +test_expect_success 'append: specify separator with line break' '
     +	test_when_finished git notes remove HEAD &&
     +	cat >expect <<-\EOF &&
    @@ t/t3301-notes.sh: test_expect_success 'listing non-existing notes fails' '
     +
     +	echo "f-notes-1" >note_a &&
     +	echo "f-notes-2" >note_b &&
    -+	git notes append -m "m-notes-1" -F note_a -m "m-notes-2" -F note_b -m "m-notes-3" --separator="-------" &&
    ++	git notes append -m "m-notes-1" -F note_a -m "m-notes-2" -F note_b -m "m-notes-3" --separator "-------" &&
     +	git notes show >actual &&
     +	test_cmp expect-combine_m_and_F actual
     +'
5:  76c93f19 ! 5:  4ba62a48 notes.c: append separator instead of insert by pos
    @@ builtin/notes.c: static void write_note_data(struct note_data *d, struct object_
     -static void insert_separator(struct strbuf *message, size_t pos)
     +static void append_separator(struct strbuf *message)
      {
    - 	if (!separator)
    - 		return;
    - 	else if (separator[strlen(separator) - 1] == '\n')
    + 	size_t sep_len = strlen(separator);
    + 	if (sep_len && separator[sep_len - 1] == '\n')
     -		strbuf_insertstr(message, pos, separator);
     +		strbuf_addstr(message, separator);
      	else
    @@ builtin/notes.c: static void write_note_data(struct note_data *d, struct object_
      
      static void concat_messages(struct note_data *d)
     @@ builtin/notes.c: static void concat_messages(struct note_data *d)
    - 
    + 	size_t i;
      	for (i = 0; i < d->msg_nr ; i++) {
      		if (d->buf.len)
     -			insert_separator(&d->buf, d->buf.len);
6:  d65f067c ! 6:  19865941 notes.c: introduce "--[no-]stripspace" option
    @@ Documentation/git-notes.txt: SYNOPSIS
      --------
      [verse]
      'git notes' [list [<object>]]
    --'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    -+'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    +-'git notes' add [-f] [--allow-empty] [--separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    ++'git notes' add [-f] [--allow-empty] [--separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
      'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )
    --'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    +-'git notes' append [--allow-empty] [--separator=<paragraph-break>] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
     -'git notes' edit [--allow-empty] [<object>]
    -+'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    ++'git notes' append [--allow-empty] [--separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
     +'git notes' edit [--allow-empty] [<object>] [--[no-]stripspace]
      'git notes' show [<object>]
      'git notes' merge [-v | -q] [-s <strategy> ] <notes-ref>
    @@ Documentation/git-notes.txt: OPTIONS
      -c <object>::
      --reedit-message=<object>::
     @@ Documentation/git-notes.txt: OPTIONS
    - 	separators will be added between paragraphs.  Defaults to a blank
    - 	line.
    + 	(a newline is added at the end as needed).  Defaults to a
    + 	blank line.
      
     +--[no-]stripspace::
     +	Strip leading and trailing whitespace from the note message.
    @@ Documentation/git-notes.txt: OPTIONS
      	`GIT_NOTES_REF` and the "core.notesRef" configuration.  The ref
     
      ## builtin/notes.c ##
    -@@
    - static const char *separator = "\n";
    - static const char * const git_notes_usage[] = {
    - 	N_("git notes [--ref <notes-ref>] [list [<object>]]"),
    --	N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    -+	N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [--[no-]stripspace] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    - 	N_("git notes [--ref <notes-ref>] copy [-f] <from-object> <to-object>"),
    --	N_("git notes [--ref <notes-ref>] append [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    -+	N_("git notes [--ref <notes-ref>] append [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [--[no-]stripspace] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"),
    - 	N_("git notes [--ref <notes-ref>] edit [--allow-empty] [<object>]"),
    - 	N_("git notes [--ref <notes-ref>] show [<object>]"),
    - 	N_("git notes [--ref <notes-ref>] merge [-v | -q] [-s <strategy>] <notes-ref>"),
     @@ builtin/notes.c: static const char * const git_notes_get_ref_usage[] = {
      static const char note_template[] =
      	N_("Write/edit the notes for the following object:");
    @@ builtin/notes.c: static int add(int argc, const char **argv, const char *prefix)
      	struct option options[] = {
      		OPT_CALLBACK_F('m', "message", &d, N_("message"),
     @@ builtin/notes.c: static int add(int argc, const char **argv, const char *prefix)
    - 			N_("<paragraph-break>"),
    - 			N_("insert <paragraph-break> between paragraphs"),
    - 			PARSE_OPT_OPTARG, parse_separator_arg),
    + 		OPT__FORCE(&force, N_("replace existing notes"), PARSE_OPT_NOCOMPLETE),
    + 		OPT_STRING(0, "separator", &separator, N_("separator"),
    + 			N_("insert <paragraph-break> between paragraphs")),
     +		OPT_BOOL(0, "stripspace", &d.stripspace,
     +			N_("remove unnecessary whitespace")),
      		OPT_END()
    @@ builtin/notes.c: static int append_edit(int argc, const char **argv, const char
      		OPT_CALLBACK_F('m', "message", &d, N_("message"),
      			N_("note contents as a string"), PARSE_OPT_NONEG,
     @@ builtin/notes.c: static int append_edit(int argc, const char **argv, const char *prefix)
    - 			N_("<paragraph-break>"),
    - 			N_("insert <paragraph-break> between paragraphs"),
    - 			PARSE_OPT_OPTARG, parse_separator_arg),
    + 			N_("allow storing empty note")),
    + 		OPT_STRING(0, "separator", &separator, N_("separator"),
    + 			N_("insert <paragraph-break> between paragraphs")),
     +		OPT_BOOL(0, "stripspace", &d.stripspace,
     +			N_("remove unnecessary whitespace")),
      		OPT_END()
-:  -------- > 7:  7105778c notes: introduce "--no-separator" option
-- 
2.40.0.356.g367cb1d4