Re: [PATCH v2 1/3] notes.c: introduce "--blank-line" option

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Nov 07 2022, Teng Long wrote:

> When appending to a given object which has note and if the appended
> note is not empty too, we will insert a blank line at first. The
> blank line serves as a split line, but sometimes we just want to
> omit it then append on the heels of the target note. So, we add
> a new "OPT_BOOL()" option to determain whether a new blank line
> is insert at first.
>
> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  Documentation/git-notes.txt | 11 +++++++++--
>  builtin/notes.c             |  5 ++++-
>  t/t3301-notes.sh            | 12 ++++++++++++
>  3 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index efbc10f0f5..43770ddf84 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] [--blank-line] [-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,9 @@ 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.
>  
>  edit::
>  	Edit the notes for a given object (defaults to HEAD).
> @@ -159,6 +161,11 @@ OPTIONS
>  	Allow an empty note object to be stored. The default behavior is
>  	to automatically remove empty notes.
>  
> +--blank-line::
> +--no-blank-line::
> +	Controls if a blank line to split paragraphs is inserted
> +	when appending (the default is true).

Just make this:

	--no-blank-line:
		Suppress the insertion of a blank line before the
		inserted notes.

Or something, i.e. when adding a "true by default" let's add a "no-..." variant directly.

>  	int allow_empty = 0;
> +	int blankline = 1;

So keep this...

>  	const char *object_ref;
>  	struct notes_tree *t;
>  	struct object_id object, new_note;
> @@ -584,6 +585,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_BOOL(0, "blank-line", &blankline,

...and just make this "no-blank-line", and parse_options() will do the
right thing. I.e. PARSE_OPT_NONEG is implied.

> -		if (d.buf.len && prev_buf && size)
> +		if (blankline && d.buf.len && prev_buf && size)
>  			strbuf_insertstr(&d.buf, 0, "\n");

Maybe this needs to be elaborated in the docs? I.e. it sounds as if
we'll insert a \n unconditionally, which this shows isn't the case.

>  		if (prev_buf && size)
>  			strbuf_insert(&d.buf, 0, prev_buf, size);
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 3288aaec7d..76beafdeb8 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -521,12 +521,24 @@ test_expect_success 'listing non-existing notes fails' '
>  	test_must_be_empty actual
>  '
>  
> +test_expect_success 'append to existing note without a beginning blank line' '
> +	cat >expect <<-\EOF &&
> +		Initial set of notes
> +		Appended notes
> +	EOF
> +	git notes add -m "Initial set of notes" &&
> +	git notes append --no-blank-line -m "Appended notes" &&
> +	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
>  
>  		More notes appended with git notes append
>  	EOF
> +	git notes remove HEAD &&

This should be a test_when_finished "", for the previous test, otherwise
this one will presumably fail if you use the "wrong" --run="" arguments
to skip the last test.