[PATCH v10 0/6] notes.c: introduce "--separator" option

[Teng Long <dyroneteng@gmail.com>]

From: Teng Long <dyroneteng@gmail.com>

Diff since v9:

1. [4/6] support `--no-separator` which means not to add any paragraph-breaks.
2. [4/6] Fix the problems by the Junio's suggestion [1]

[1] https://public-inbox.org/git/xmqqsfcjbuud.fsf@gitster.g/

Thanks.

Teng Long (6):
  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 '[--[no-]separator|--separator=<paragraph-break>]'
    option
  notes.c: append separator instead of insert by pos
  notes.c: introduce "--[no-]stripspace" option

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

Range-diff against v9:
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:  ed930ef4 ! 4:  820dda04 notes.c: introduce '--separator=<paragraph-break>' option
    @@ Metadata
     Author: Teng Long <dyroneteng@gmail.com>
     
      ## Commit message ##
    -    notes.c: introduce '--separator=<paragraph-break>' option
    +    notes.c: introduce '[--[no-]separator|--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] [--separator=<paragraph-break>] [-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' 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] [--separator=<paragraph-break>] [-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' 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.
      
    -+--separator <paragraph-break>::
    ++--[no-]separator, --separator=<paragraph-break>::
     +	Specify a string used as a custom inter-paragraph separator
    -+	(a newline is added at the end as needed).  Defaults to a
    -+	blank line.
    ++	(a newline is added at the end as needed). If `--no-separator`, no
    ++	separators will be added between paragraphs.  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 char *separator = "\n";
    ++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] [-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>] add [-f] [--allow-empty] [--[no-]separator|--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] [--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>] [-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[strlen(separator) - 1] == '\n')
    -+		strbuf_addstr(message, separator);
    ++	if (!separator)
    ++		return;
    ++	else if (separator[strlen(separator) - 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;
    ++
     +	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_STRING(0, "separator", &separator, N_("separator"),
    -+			N_("insert <paragraph-break> between paragraphs")),
    ++		OPT_CALLBACK_F(0, "separator", &separator,
    ++			N_("<paragraph-break>"),
    ++			N_("insert <paragraph-break> between paragraphs"),
    ++			PARSE_OPT_OPTARG, parse_separator_arg),
      		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_STRING(0, "separator", &separator, N_("separator"),
    -+			N_("insert <paragraph-break> between paragraphs")),
    ++		OPT_CALLBACK_F(0, "separator", &separator,
    ++			N_("<paragraph-break>"),
    ++			N_("insert <paragraph-break> between paragraphs"),
    ++			PARSE_OPT_OPTARG, parse_separator_arg),
      		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 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 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 an empty separator' '
    ++test_expect_success 'append: specify a separator with an empty arg' '
     +	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:  eea2246f ! 5:  76c93f19 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[strlen(separator) - 1] == '\n')
    - 		strbuf_addstr(message, separator);
    + 	if (!separator)
    + 		return;
    + 	else if (separator[strlen(separator) - 1] == '\n')
    +-		strbuf_insertstr(message, pos, separator);
    ++		strbuf_addstr(message, separator);
      	else
     -		strbuf_insertf(message, pos, "%s%s", separator, "\n");
     +		strbuf_addf(message, "%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)
     @@ 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:  20063bea ! 6:  d65f067c notes.c: introduce "--[no-]stripspace" option
    @@ Documentation/git-notes.txt: SYNOPSIS
      --------
      [verse]
      'git notes' [list [<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' 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' copy [-f] ( --stdin | <from-object> [<to-object>] )
    --'git notes' append [--allow-empty] [--separator=<paragraph-break>] [-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' edit [--allow-empty] [<object>]
    -+'git notes' append [--allow-empty] [--separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
    ++'git notes' append [--allow-empty] [--[no-]separator | --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
    - 	(a newline is added at the end as needed).  Defaults to a
    - 	blank line.
    + 	separators will be added between paragraphs.  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)
    - 		OPT__FORCE(&force, N_("replace existing notes"), PARSE_OPT_NOCOMPLETE),
    - 		OPT_STRING(0, "separator", &separator, N_("separator"),
    - 			N_("insert <paragraph-break> between paragraphs")),
    + 			N_("<paragraph-break>"),
    + 			N_("insert <paragraph-break> between paragraphs"),
    + 			PARSE_OPT_OPTARG, parse_separator_arg),
     +		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_("allow storing empty note")),
    - 		OPT_STRING(0, "separator", &separator, N_("separator"),
    - 			N_("insert <paragraph-break> between paragraphs")),
    + 			N_("<paragraph-break>"),
    + 			N_("insert <paragraph-break> between paragraphs"),
    + 			PARSE_OPT_OPTARG, parse_separator_arg),
     +		OPT_BOOL(0, "stripspace", &d.stripspace,
     +			N_("remove unnecessary whitespace")),
      		OPT_END()
-- 
2.40.0.356.g67a1c1d0