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

Re: [PATCH v3 2/2] format-patch: teach format.notes config option

From
Junio C Hamano <gitster@pobox.com>
Date
May 13, 2019, 02:44 UTC
Message-ID
<xmqqimufdqey.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<df864c4adf4dcab5f959007f87b1c4d0eafecb52.1557513353.git.liu.denton@gmail.com>
Denton Liu <liu.denton@gmail.com> writes:
Show 18 quoted lines
> @@ -864,6 +866,20 @@ static int git_format_config(const char *var, const char *value, void *cb)
>  			from = NULL;
>  		return 0;
>  	}
> +	if (!strcmp(var, "format.notes")) {
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		rev->show_notes = 1;
> +		if (!strcmp(value, "standard")) {
> +			rev->notes_opt.use_default_notes = 1;
> +		} else {
> +			strbuf_addstr(&buf, value);
> +			expand_notes_ref(&buf);
> +			string_list_append(&rev->notes_opt.extra_notes_refs,
> +					strbuf_detach(&buf, NULL));
> +		}
> +		return 0;
> +	}

Unlike the command line option parser, this does not seem to touch rev->show_notes_given at all. Intended? I am wondering how well this implementation meshes with what 66b2ed09 ("Fix "log" family not to be too agressive about showing notes", 2010-01-20) wanted to do, which 894a9d33 ("Support showing notes from more than one notes tree", 2010-03-12) later extended.

Show 16 quoted lines
> +test_expect_success 'format-patch notes output control' '
> +	git notes add -m "notes config message" HEAD &&
> +	test_when_finished git notes remove HEAD &&
> +
> +	git format-patch -1 --stdout >out &&
> +	! grep "notes config message" out &&
> +	git format-patch -1 --stdout --notes >out &&
> +	grep "notes config message" out &&
> +	git format-patch -1 --stdout --no-notes >out &&
> +	! grep "notes config message" out &&
> +	git format-patch -1 --stdout --notes --no-notes >out &&
> +	! grep "notes config message" out &&
> +	git format-patch -1 --stdout --no-notes --notes >out &&
> +	grep "notes config message" out &&
> +
> +	test_config format.notes standard &&
I think we tend to spell these things "default".

Alternatively, the format.notes configuration can be "bool or text", and make the variable set to 'true' mean "show notes, using the default ref".

Show 11 quoted lines
> +	git format-patch -1 --stdout >out &&
> +	grep "notes config message" out &&
> +	git format-patch -1 --stdout --notes >out &&
> +	grep "notes config message" out &&
> +	git format-patch -1 --stdout --no-notes >out &&
> +	! grep "notes config message" out &&
> +	git format-patch -1 --stdout --notes --no-notes >out &&
> +	! grep "notes config message" out &&
> +	git format-patch -1 --stdout --no-notes --notes >out &&
> +	grep "notes config message" out
> +'
OK.
Show 9 quoted lines
> +test_expect_success 'format-patch with multiple notes refs' '
> +	git notes --ref note1 add -m "this is note 1" HEAD &&
> +	test_when_finished git notes --ref note1 remove HEAD &&
> +	git notes --ref note2 add -m "this is note 2" HEAD &&
> +	test_when_finished git notes --ref note2 remove HEAD &&
> + ...
> +	git format-patch -1 --stdout --notes=note1 --notes=note2 >out &&
> +	grep "this is note 1" out &&
> +	grep "this is note 2" out &&
Do we promise the order in which these two lines appear in the output?
Show 10 quoted lines
> +	test_config format.notes note1 &&
> +	git format-patch -1 --stdout >out &&
> +	grep "this is note 1" out &&
> +	! grep "this is note 2" out &&
> +	git format-patch -1 --stdout --no-notes >out &&
> +	! grep "this is note 1" out &&
> +	! grep "this is note 2" out &&
> +	git format-patch -1 --stdout --notes=note2 >out &&
> +	grep "this is note 1" out &&
> +	grep "this is note 2" out &&

So format.notes say note1 but the command line explicitly asks it wants note from note2, but the command still gives from note1 anyway.

> +	git format-patch -1 --stdout --no-notes --notes=note2 >out &&
> +	! grep "this is note 1" out &&
> +	grep "this is note 2" out &&

And there is a way to work it around, i.e. clear everything configured with --no-notes and then name the one you want from the command line.

I am not sure if the above is consistent with how our options and configurations interact in general. Shouldn't the --notes=note2 alone in the earlier example cancel format.notes=note1 configured?

Show 12 quoted lines
> +	git config --add format.notes note2 &&
> +	git format-patch -1 --stdout >out &&
> +	grep "this is note 1" out &&
> +	grep "this is note 2" out &&
> +	git format-patch -1 --stdout --no-notes >out &&
> +	! grep "this is note 1" out &&
> +	! grep "this is note 2" out
> +'
> +
>  echo "fatal: --name-only does not make sense" > expect.name-only
>  echo "fatal: --name-status does not make sense" > expect.name-status
>  echo "fatal: --check does not make sense" > expect.check
Previous: Denton LiuNext: Denton Liu
Message 14 of 15 in “format-patch: teach format.notes config option”
  1. 0/2 format-patch: teach format.notes config optionDenton Liu, Apr 27, 2019
  2. 1/2 git-format-patch.txt: document --no-notes optionDenton Liu, Apr 27, 2019
  3. 2/2 format-patch: teach format.notes config optionDenton Liu, Apr 27, 2019
  4. Junio C HamanoMay 7, 2019
  5. 0/2 format-patch: teach format.notes config optionDenton Liu, May 8, 2019
  6. 1/2 git-format-patch.txt: document --no-notes optionDenton Liu, May 8, 2019
  7. 2/2 format-patch: teach format.notes config optionDenton Liu, May 8, 2019
  8. Beat BolliMay 8, 2019
  9. Denton LiuMay 8, 2019
  10. Beat BolliMay 8, 2019
  11. 0/2 format-patch: teach format.notes config optionDenton Liu, May 10, 2019
  12. 1/2 git-format-patch.txt: document --no-notes optionDenton Liu, May 10, 2019
  13. 2/2 format-patch: teach format.notes config optionDenton Liu, May 10, 2019
  14. Junio C HamanoMay 13, 2019
  15. Denton LiuMay 14, 2019

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.