git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v11 0/7] notes.c: introduce "--separator"

From
Teng Long <dyroneteng@gmail.com>
Date
May 27, 2023, 07:57 UTC
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
Previous: Teng LongNext: Teng Long
Message 46 of 55 in “notes.c: introduce "--separator" option”
  1. 0/6 notes.c: introduce "--separator" optionTeng Long, Apr 28, 2023
  2. 1/6 notes.c: cleanup 'strbuf_grow' call in 'append_edit'Teng Long, Apr 28, 2023
  3. 2/6 notes.c: use designated initializers for clarityTeng Long, Apr 28, 2023
  4. 3/6 t3321: add test cases about the notes stripspace behaviorTeng Long, Apr 28, 2023
  5. 4/6 notes.c: introduce '--separator=<paragraph-break>' optionTeng Long, Apr 28, 2023
  6. Junio C HamanoApr 28, 2023
  7. Teng LongMay 6, 2023
  8. Teng LongMay 6, 2023
  9. Kristoffer HaugsbakkMay 10, 2023
  10. Teng LongMay 12, 2023
  11. Kristoffer HaugsbakkMay 12, 2023
  12. Junio C HamanoMay 16, 2023
  13. Teng LongMay 17, 2023
  14. Junio C HamanoMay 17, 2023
  15. Junio C HamanoJun 14, 2023
  16. notes: do not access before the beginning of an arrayJunio C Hamano, Jun 14, 2023
  17. Eric SunshineJun 14, 2023
  18. Junio C HamanoJun 14, 2023
  19. Jeff KingJun 15, 2023
  20. Junio C HamanoJun 15, 2023
  21. Teng LongJun 19, 2023
  22. Junio C HamanoJun 20, 2023
  23. Teng LongJun 21, 2023
  24. 5/6 notes.c: append separator instead of insert by posTeng Long, Apr 28, 2023
  25. 6/6 notes.c: introduce "--[no-]stripspace" optionTeng Long, Apr 28, 2023
  26. Junio C HamanoApr 28, 2023
  27. Junio C HamanoMay 1, 2023
  28. 0/6 notes.c: introduce "--separator" optionTeng Long, May 18, 2023
  29. 1/6 notes.c: cleanup 'strbuf_grow' call in 'append_edit'Teng Long, May 18, 2023
  30. 2/6 notes.c: use designated initializers for clarityTeng Long, May 18, 2023
  31. 3/6 t3321: add test cases about the notes stripspace behaviorTeng Long, May 18, 2023
  32. 5/6 notes.c: append separator instead of insert by posTeng Long, May 18, 2023
  33. 4/6 notes.c: introduce '[--[no-]separator|--separator=<paragraph-break>]' optionTeng Long, May 18, 2023
  34. Kristoffer HaugsbakkMay 18, 2023
  35. Teng LongMay 20, 2023
  36. Kristoffer HaugsbakkMay 20, 2023
  37. Jeff KingMay 19, 2023
  38. Teng LongMay 27, 2023
  39. Jeff KingMay 27, 2023
  40. Teng LongMay 29, 2023
  41. 6/6 notes.c: introduce "--[no-]stripspace" optionTeng Long, May 18, 2023
  42. Kristoffer HaugsbakkMay 18, 2023
  43. Teng LongMay 20, 2023
  44. Junio C HamanoMay 18, 2023
  45. Teng LongMay 20, 2023
  46. 0/7 notes.c: introduce "--separator"Teng Long, May 27, 2023
  47. 1/7 notes.c: cleanup 'strbuf_grow' call in 'append_edit'Teng Long, May 27, 2023
  48. 2/7 notes.c: use designated initializers for clarityTeng Long, May 27, 2023
  49. 3/7 t3321: add test cases about the notes stripspace behaviorTeng Long, May 27, 2023
  50. 4/7 notes.c: introduce '--separator=<paragraph-break>' optionTeng Long, May 27, 2023
  51. 5/7 notes.c: append separator instead of insert by posTeng Long, May 27, 2023
  52. 6/7 notes.c: introduce "--[no-]stripspace" optionTeng Long, May 27, 2023
  53. 7/7 notes: introduce "--no-separator" optionTeng Long, May 27, 2023
  54. Junio C HamanoJun 1, 2023
  55. Teng LongJun 3, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.