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

[Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>]

On Tue, Nov 08 2022, Teng Long wrote:

> "Ævar Arnfjörð Bjarmason" <avarab@xxxxxxxxx> writes:
>
>> >  	int allow_empty = 0;
>> > +	int blankline = 1;
>>
>> So keep this...
>>
>> >  	const char *object_ref;
>> >G  	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.
>
> Sorry for another question. By the explanation of "api-parse-options.txt" :
>
> `OPT_BOOL(short, long, &int_var, description)`::
> 	Introduce a boolean option. `int_var` is set to one with
> 	`--option` and set to zero with `--no-option`
>
> I think it means the same as "parse_options() will do right thing" as you said
> to me , but...after the modification the effect is opposite:
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index a6273781fb8..73427ea8dff 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -562,6 +562,7 @@ static int copy(int argc, const char **argv, const char *prefix)
>  static int append_edit(int argc, const char **argv, const char *prefix)
>  {
>         int allow_empty = 0;
> +       int blankline = 1;
>         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, "no-blank-line", &blankline,
> +                       N_("insert paragraph break before appending to an existing note")),
>                 OPT_END()
>         };
>         int edit = !strcmp(argv[0], "edit");
> @@ -618,7 +621,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
>                 enum object_type type;
>                 char *prev_buf = read_object_file(note, &type, &size);
>
> -               if (d.buf.len && prev_buf && size)
> +               if (blankline && d.buf.len && prev_buf && size)
>                         strbuf_insertstr(&d.buf, 0, "\n");
>                 if (prev_buf && size)
>                         strbuf_insert(&d.buf, 0, prev_buf, size);
>
> ----
> So, I am a bit confused and I guess maybe somewhere I misunderstood or didn't
> notice some details.
>
> Thanks.

Sorry, I meant that in both cases it will expose the same options to the
user: --blank-line and --no-blank-line. I.e. if you create options
named:

	"x" "x-y"

Their negations are: --no-x and --no-x-y. But if their names are:

	"x" "no-x"

The negations are:

	--no-x and --x

But as your example shows that's unrelated to whether the *variable in
the code* is negated.

So however you structure the code, which would be:

	int blankline = 1:
        [...]
	OPT_BOOL(0, "blankline", &blankline, [...]);

Or:

	int no_blankline = 0:
        [...]
	OPT_BOOL(0, "no-blankline", &no_blankline, [...]);

The documentation could in both cases say:

	--no-blankline:
		describe the non-default[...]