[PATCH v4 5/5] notes.c: introduce "--separator" option

[Teng Long <dyroneteng@gmail.com>]

From: Teng Long <dyroneteng@gmail.com>

When appending to a given notes object and the appended note is not
empty too, we will insert a blank line at first which separates the
existing note and the appended one, which as the separator.

Sometimes, we want to use a specified <text> as the separator. For
example, if we specify as:

    * --separator='------': we will insert "------\n" as the separator,
    because user do not provide the line break char at last, we will add
    the trailing '\n' compatibly.

    * --separator='------\n': we will insert as-is because it contains
    the line break at last.

    * --separator='': we specify an empty separator which means will
    append the message directly without inserting any separator at
    first.

    * not specified --separator option: will use '\n' as the separator
    when do appending and this is the default behavour.

Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 Documentation/git-notes.txt | 18 +++++++++--
 builtin/notes.c             | 49 +++++++++++++++++++++++++++---
 t/t3301-notes.sh            | 59 +++++++++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index efbc10f0f5..227fa88317 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git notes' [list [<object>]]
 'git notes' add [-f] [--allow-empty] [-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] [-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>
@@ -86,7 +86,11 @@ the command can read the input given to the `post-rewrite` hook.)
 
 append::
 	Append to the notes of an existing object (defaults to HEAD).
-	Creates a new notes object if needed.
+	Creates a new notes object if needed. If the note of the given
+	object and the note to be appended are not empty, a blank line
+	will be inserted between them as the separator ("blank line" is
+	the default behavior, `--separator` option supports to specify
+	a customized one).
 
 edit::
 	Edit the notes for a given object (defaults to HEAD).
@@ -159,6 +163,16 @@ OPTIONS
 	Allow an empty note object to be stored. The default behavior is
 	to automatically remove empty notes.
 
+--separator <text>::
+	Specify the <text> to be inserted between existing note and appended
+	message, the <text> acts as a separator.
+	If <text> is empty (`--separator=''`), will append the message to
+	existing note directly without insert any separator.
+	If <text> is nonempty, will use as-is. One thing to notice is if
+	the <text> lacks newline charactor, will add the newline automatically.
+	If not specify this option, a blank line will be inserted as the
+	separator.
+
 --ref <ref>::
 	Manipulate the notes tree in <ref>.  This overrides
 	`GIT_NOTES_REF` and the "core.notesRef" configuration.  The ref
diff --git a/builtin/notes.c b/builtin/notes.c
index f2efb3736c..6746ad3232 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -24,6 +24,8 @@
 #include "notes-utils.h"
 #include "worktree.h"
 
+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>]"),
@@ -209,7 +211,7 @@ static void write_note_data(struct note_data *d, struct object_id *oid)
 	}
 }
 
-static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
+static int parse_msg_arg_add(const struct option *opt, const char *arg, int unset)
 {
 	struct note_data *d = opt->value;
 
@@ -225,6 +227,43 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static void insert_separator(struct strbuf *message)
+{
+	const char *insert;
+
+	if (!separator)
+		separator = "\n";
+	if (*separator == '\0')
+		/* separator is empty; use as-is (no blank line) */
+		return;
+	else if (separator[strlen(separator) - 1] == '\n')
+		/* user supplied newline; use as-is */
+		insert = separator;
+	else
+		/* separator lacks newline; add it ourselves */
+		insert = xstrfmt("%s%s", separator,"\n");
+	strbuf_insertstr(message, 0, insert);
+}
+
+static int parse_msg_arg_append(const struct option *opt, const char *arg, int unset)
+{
+	struct note_data *d = opt->value;
+	struct strbuf append = STRBUF_INIT;
+
+	BUG_ON_OPT_NEG(unset);
+
+	strbuf_addstr(&append, arg);
+	if (d->buf.len){
+		insert_separator(&append);
+	}
+	strbuf_addbuf(&d->buf, &append);
+	strbuf_stripspace(&d->buf, 0);
+
+	d->given = 1;
+	strbuf_release(&append);
+	return 0;
+}
+
 static int parse_file_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct note_data *d = opt->value;
@@ -406,7 +445,7 @@ static int add(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_CALLBACK_F('m', "message", &d, N_("message"),
 			N_("note contents as a string"), PARSE_OPT_NONEG,
-			parse_msg_arg),
+			parse_msg_arg_add),
 		OPT_CALLBACK_F('F', "file", &d, N_("file"),
 			N_("note contents in a file"), PARSE_OPT_NONEG,
 			parse_file_arg),
@@ -572,7 +611,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_CALLBACK_F('m', "message", &d, N_("message"),
 			N_("note contents as a string"), PARSE_OPT_NONEG,
-			parse_msg_arg),
+			parse_msg_arg_append),
 		OPT_CALLBACK_F('F', "file", &d, N_("file"),
 			N_("note contents in a file"), PARSE_OPT_NONEG,
 			parse_file_arg),
@@ -584,6 +623,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 			parse_reuse_arg),
 		OPT_BOOL(0, "allow-empty", &allow_empty,
 			N_("allow storing empty note")),
+		OPT_STRING(0, "separator", &separator, N_("text"),
+			N_("insert <text> as separator before appending to an existing note")),
 		OPT_END()
 	};
 	int edit = !strcmp(argv[0], "edit");
@@ -619,7 +660,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 		char *prev_buf = read_object_file(note, &type, &size);
 
 		if (d.buf.len && prev_buf && size)
-			strbuf_insertstr(&d.buf, 0, "\n");
+			insert_separator(&d.buf);
 		if (prev_buf && size)
 			strbuf_insert(&d.buf, 0, prev_buf, size);
 		free(prev_buf);
git format-patch a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1  --thread -v 4 --output-directory=outgoing/git-notes-append/v4 --cover-letter  --range-diff 196e80358ediff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index e7807e052a..e8bc9934ed 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -521,6 +521,65 @@ test_expect_success 'listing non-existing notes fails' '
 	test_must_be_empty actual
 '
 
+test_expect_success 'append: specify an empty 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 --separator="" -m "notes-2" &&
+	git notes show >actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'append: specify separatoro with line break' '
+	test_when_finished git notes remove HEAD &&
+	cat >expect <<-\EOF &&
+		notes-1
+		-------
+		notes-2
+	EOF
+
+	git notes add -m "notes-1" &&
+	separator=$(printf "%s\n" "-------") &&
+	git notes append --separator="$separator" -m "notes-2" &&
+	git notes show >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'append: specify separator without line break' '
+	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 separator with multiple messages' '
+	test_when_finished git notes remove HEAD &&
+	cat >expect <<-\EOF &&
+		notes-1
+		-------
+		notes-2
+		-------
+		notes-3
+	EOF
+
+	git notes add -m "notes-1" &&
+	git notes append --separator="-------" -m "notes-2" -m "notes-3" &&
+	git notes show >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'append to existing note with "git notes append"' '
 	cat >expect <<-EOF &&
 		Initial set of notes
-- 
2.38.1.386.g6952793f2d9.dirty