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

Re: [PATCH 2/2] add -i: default to the built-in implementation

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Dec 2, 2021, 17:30 UTC
Message-ID
<nycvar.QRO.7.76.6.2112021635330.63@tvgsbejvaqbjf.bet>
In-Reply-To
<xmqqee6wymx2.fsf@gitster.g>
Hi Junio,
On Wed, 1 Dec 2021, Junio C Hamano wrote:
Show 38 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> [...]
> > diff --git a/builtin/add.c b/builtin/add.c
> > index ef6b619c45e..8ef230a345b 100644
> > --- a/builtin/add.c
> > +++ b/builtin/add.c
> > @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,
> >  	int use_builtin_add_i =
> >  		git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1);
> >
> > -	if (use_builtin_add_i < 0) {
> > -		int experimental;
> > -		if (!git_config_get_bool("add.interactive.usebuiltin",
> > -					 &use_builtin_add_i))
> > -			; /* ok */
> > -		else if (!git_config_get_bool("feature.experimental", &experimental) &&
> > -			 experimental)
> > -			use_builtin_add_i = 1;
> > -	}
> > +	if (use_builtin_add_i < 0 &&
> > +	    git_config_get_bool("add.interactive.usebuiltin",
> > +				&use_builtin_add_i))
> > +		use_builtin_add_i = 1;
> >
> > -	if (use_builtin_add_i == 1) {
> > +	if (use_builtin_add_i != 0) {
>
> Nit.
>
> 	if (use_builtin_add_i) {
>
> I wondered if these random calls to git_config_get_X() should be
> consolidated into the existing add_config() callback, but this
> conditional will go away hopefully in a few releases, so it probably
> is not worth it.  Inheriting the way the original code grabbed the
> configuration variables is good enough for our purpose here.

As I said in my reply to Phillip, I found it misleading to skip the `!= 0` because we are catching both the `== 1` as well as the `== -1` here.

Ciao, Dscho

Previous: Junio C HamanoNext: Junio C Hamano
Message 14 of 23 in “Use the built-in implementation of the interactive add command by default”
  1. 0/2 Use the built-in implementation of the interactive add command by defaultJohannes Schindelin via GitGitGadget, Nov 30, 2021
  2. 1/2 t2016: require the PERL prereq only when necessaryJohannes Schindelin via GitGitGadget, Nov 30, 2021
  3. Junio C HamanoDec 1, 2021
  4. Johannes SchindelinDec 2, 2021
  5. 2/2 add -i: default to the built-in implementationJohannes Schindelin via GitGitGadget, Nov 30, 2021
  6. Phillip WoodDec 1, 2021
  7. Johannes SchindelinDec 2, 2021
  8. Junio C HamanoDec 2, 2021
  9. Ramsay JonesDec 2, 2021
  10. Johannes SchindelinDec 10, 2021
  11. Ævar Arnfjörð BjarmasonDec 1, 2021
  12. Junio C HamanoDec 1, 2021
  13. Junio C HamanoDec 1, 2021
  14. Johannes SchindelinDec 2, 2021
  15. fixup! add -i: default to the built-in implementationJunio C Hamano, Dec 9, 2021
  16. Johannes SchindelinDec 10, 2021
  17. Jeff KingNov 30, 2021
  18. Junio C HamanoNov 30, 2021
  19. Ævar Arnfjörð BjarmasonDec 1, 2021
  20. Carlo ArenasDec 1, 2021
  21. Johannes SchindelinDec 2, 2021
  22. Philippe BlainDec 3, 2021
  23. Johannes SchindelinDec 6, 2021

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.