{"thread":{"id":"55935","subject":"[PATCH] doc: builtin add -i is enabled by feature.experimental","startedAt":"2021-06-15T16:45:29Z","lastAt":"2021-06-17T12:01:38Z","messageCount":5,"participants":["Todd Zullinger","Ævar Arnfjörð Bjarmason","Junio C Hamano","Derrick Stolee","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"427500","messageId":"20210615164522.1079951-1-tmz@pobox.com","threadId":"55935","inReplyTo":null,"subject":"[PATCH] doc: builtin add -i is enabled by feature.experimental","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2021-06-15T16:45:22Z","receivedAt":"2021-06-15T16:45:29Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Note that add.interactive.useBuiltin is enabled by feature.experimental.\nIt was added in 2df2d81ddd (add -i: use the built-in version when\nfeature.experimental is set, 2020-09-08).\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\nI was checking my configuration to see if I still needed to have\nadd.interactive.useBuiltin set and noticed that it wasn't listed in the\nsettings enabled by feature.experimental.\n\nUnless it's time to take this out of the experimental phase, it seems worth\ndocumenting -- if for no other reason than to keep fetch.negotiationAlgorithm\nfrom being lonely in the feature.experimental section. ;)\n\nFWIW, I've had this enabled for perhaps a year or so and I don't recall any\ninstances where it hasn't worked well.  It has come in handy on some systems\nwhere I can't easily install Perl but don't want to give up `add -[ip]`.\nThanks Dscho!\n\n Documentation/config/add.txt     | 3 ++-\n Documentation/config/feature.txt | 6 ++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\nindex c9f748f81c..7d6d325571 100644\n--- a/Documentation/config/add.txt\n+++ b/Documentation/config/add.txt\n@@ -9,4 +9,5 @@ add.ignore-errors (deprecated)::\n add.interactive.useBuiltin::\n \t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n \timplementation of the interactive version of linkgit:git-add[1]\n-\tinstead of the Perl script version. Is `false` by default.\n+\tinstead of the Perl script version.  If `feature.experimental` is\n+\tenabled, this setting is `true`.  By default, it is `false`.\ndiff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt\nindex cdecd04e5b..caaa97dc61 100644\n--- a/Documentation/config/feature.txt\n+++ b/Documentation/config/feature.txt\n@@ -12,6 +12,10 @@ feature.experimental::\n \tsetting if you are interested in providing feedback on experimental\n \tfeatures. The new default values are:\n +\n+* `add.interactive.useBuiltin=true` which enables the built-in implementation\n+of the interactive version of linkgit:git-add[1] instead of the Perl script\n+version.\n++\n * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by\n skipping more commits at a time, reducing the number of round trips.\n \n feature.manyFiles::\n \tEnable config options that optimize for repos with many files in the\n-- \n2.32.0\n\n"},{"id":"427523","messageId":"87v96fx74n.fsf@evledraar.gmail.com","threadId":"55935","inReplyTo":"20210615164522.1079951-1-tmz@pobox.com","subject":"Re: [PATCH] doc: builtin add -i is enabled by feature.experimental","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-15T19:24:16Z","receivedAt":"2021-06-15T19:25:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jun 15 2021, Todd Zullinger wrote:\n\n> Note that add.interactive.useBuiltin is enabled by feature.experimental.\n> It was added in 2df2d81ddd (add -i: use the built-in version when\n> feature.experimental is set, 2020-09-08).\n>\n> Signed-off-by: Todd Zullinger <tmz@pobox.com>\n> ---\n> I was checking my configuration to see if I still needed to have\n> add.interactive.useBuiltin set and noticed that it wasn't listed in the\n> settings enabled by feature.experimental.\n>\n> Unless it's time to take this out of the experimental phase, it seems worth\n> documenting -- if for no other reason than to keep fetch.negotiationAlgorithm\n> from being lonely in the feature.experimental section. ;)\n>\n> FWIW, I've had this enabled for perhaps a year or so and I don't recall any\n> instances where it hasn't worked well.  It has come in handy on some systems\n> where I can't easily install Perl but don't want to give up `add -[ip]`.\n> Thanks Dscho!\n>\n>  Documentation/config/add.txt     | 3 ++-\n>  Documentation/config/feature.txt | 6 ++++++\n>  2 files changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\n> index c9f748f81c..7d6d325571 100644\n> --- a/Documentation/config/add.txt\n> +++ b/Documentation/config/add.txt\n> @@ -9,4 +9,5 @@ add.ignore-errors (deprecated)::\n>  add.interactive.useBuiltin::\n>  \t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n>  \timplementation of the interactive version of linkgit:git-add[1]\n> -\tinstead of the Perl script version. Is `false` by default.\n> +\tinstead of the Perl script version.  If `feature.experimental` is\n> +\tenabled, this setting is `true`.  By default, it is `false`.\n> diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt\n> index cdecd04e5b..caaa97dc61 100644\n> --- a/Documentation/config/feature.txt\n> +++ b/Documentation/config/feature.txt\n> @@ -12,6 +12,10 @@ feature.experimental::\n>  \tsetting if you are interested in providing feedback on experimental\n>  \tfeatures. The new default values are:\n>  +\n> +* `add.interactive.useBuiltin=true` which enables the built-in implementation\n> +of the interactive version of linkgit:git-add[1] instead of the Perl script\n> +version.\n> ++\n>  * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by\n>  skipping more commits at a time, reducing the number of round trips.\n>  \n>  feature.manyFiles::\n>  \tEnable config options that optimize for repos with many files in the\n\nUnless there's specific bugs in it that I'm unaware of, I would think\nthat we'd be better of just removing the setting and switching 100% to\nthe built-in. See the history of the other *.useBuiltin settings, where\nwe migrated on a fairly fast schedule.\n\nI also see that use of stash.useBuiltin is still warning for a few\nreleases now, that could be removed entirely while we're at it. See\n9bcde4d5314 (rebase: remove transitory rebase.useBuiltin setting & env,\n2021-03-23).\n"},{"id":"427565","messageId":"xmqq4kdyturm.fsf@gitster.g","threadId":"55935","inReplyTo":"20210615164522.1079951-1-tmz@pobox.com","subject":"Re: [PATCH] doc: builtin add -i is enabled by feature.experimental","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-16T02:20:45Z","receivedAt":"2021-06-16T02:20:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Note that add.interactive.useBuiltin is enabled by feature.experimental.\n> It was added in 2df2d81ddd (add -i: use the built-in version when\n> feature.experimental is set, 2020-09-08).\n>\n> Signed-off-by: Todd Zullinger <tmz@pobox.com>\n\n> ---\n> I was checking my configuration to see if I still needed to have\n> add.interactive.useBuiltin set and noticed that it wasn't listed in the\n> settings enabled by feature.experimental.\n\nSo together with the fetch.negotiationAlgorithm only two knobs are\naffected by feature.experimental?  Or is this patch only about add-i\nbecause that is the only thing you found out about?\n\nExplicitly state that these are the only two that are affected by\nthis bit in the log message so that readers of \"git log\" do not have\nto ask the question.\n\nThe other configuration feature.experimental controls is described\nin Documentation/config/fetch.txt like this:\n\n    fetch.negotiationAlgorithm::\n            Control how information about the commits in the local repository is\n            sent when negotiating the contents of the packfile to be sent by the\n            ...\n            The default is \"default\" which instructs Git to use the default algorithm\n            that never skips commits (unless the server has acknowledged it or one\n            of its descendants). If `feature.experimental` is enabled, then this\n            setting defaults to \"skipping\".\n            Unknown values will cause 'git fetch' to error out.\n\n> diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\n> index c9f748f81c..7d6d325571 100644\n> --- a/Documentation/config/add.txt\n> +++ b/Documentation/config/add.txt\n> @@ -9,4 +9,5 @@ add.ignore-errors (deprecated)::\n>  add.interactive.useBuiltin::\n>  \t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n>  \timplementation of the interactive version of linkgit:git-add[1]\n> -\tinstead of the Perl script version. Is `false` by default.\n> +\tinstead of the Perl script version.  If `feature.experimental` is\n> +\tenabled, this setting is `true`.  By default, it is `false`.\n\nThe added sentence doesn't exactly make sense.  If the experimental\nbit is set, this setting _defaults_ to true.  If you set this\nexplicitly, the experimental bit would not override it, would it?\n\nI prefer to see things described only once, so I am wondering if we\ncan get away by doing these instead:\n\n - Start the description of fetch.negotiationAlgorithm with\n   [EXPERIMENTAL] like this one does, and remove the sentence about\n   the experimental bit from there.\n\n - Leave the description of add.interactive.useBuiltin as is.\n\n - Use the change you made to Documentation/config/feature.txt below.\n\nAdditionally, it may help readers without hurting maintainability to\nsay \"[EXPERIMENTAL] (see also `feature.experimental`)\" to refer them\nto the description of the defaults set by the experimental bit.\n\nAlternatively, we can \n\n - Remove the description of other configuration variables affected\n   by feature.experimental from the description of the experimental\n   bit in Documentation/config/feature.txt, and replace it with \"The\n   default values for configuration variables marked with\n   '[EXPERIMENTAL]' are affected by setting this bit\", and\n\n - Start the description of fetch.negotiationAlgorithm with\n   [EXPERIMENTAL] like this one does, and remove the sentence about\n   the experimental bit from there.\n\n - Use the change you made to Documentation/config/add.txt as is.\n\nThat would also reduce duplicates.  From organizational point of\nview, I prefer the latter slightly more.  It makes the readers who\nwant to learn what knobs would be tweaked to run \"grep\" on the\ndocumentation set, but it may not be too much to expect from the\nmore adventuous types.  I dunno.\n\nI am sending this also to Derrick (for adding the experimental bit\nand threw negotiate knob into it) and Jonathan Tan (for adding and\ndescribing negotiate) for their input.\n\nThanks.\n\n> diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt\n> index cdecd04e5b..caaa97dc61 100644\n> --- a/Documentation/config/feature.txt\n> +++ b/Documentation/config/feature.txt\n> @@ -12,6 +12,10 @@ feature.experimental::\n>  \tsetting if you are interested in providing feedback on experimental\n>  \tfeatures. The new default values are:\n>  +\n> +* `add.interactive.useBuiltin=true` which enables the built-in implementation\n> +of the interactive version of linkgit:git-add[1] instead of the Perl script\n> +version.\n> ++\n>  * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by\n>  skipping more commits at a time, reducing the number of round trips.\n>  \n>  feature.manyFiles::\n>  \tEnable config options that optimize for repos with many files in the\n"},{"id":"427623","messageId":"545a93c8-3135-6cc5-6b14-1597a2658b9d@gmail.com","threadId":"55935","inReplyTo":"xmqq4kdyturm.fsf@gitster.g","subject":"Re: [PATCH] doc: builtin add -i is enabled by feature.experimental","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-06-16T17:38:46Z","receivedAt":"2021-06-16T17:38:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/15/2021 10:20 PM, Junio C Hamano wrote:\n> Todd Zullinger <tmz@pobox.com> writes:\n> \n>> Note that add.interactive.useBuiltin is enabled by feature.experimental.\n>> It was added in 2df2d81ddd (add -i: use the built-in version when\n>> feature.experimental is set, 2020-09-08).\n\nThank you for noticing that there is discrepancy in the docs.\n\n> So together with the fetch.negotiationAlgorithm only two knobs are\n> affected by feature.experimental?  Or is this patch only about add-i\n> because that is the only thing you found out about?\n> \n> Explicitly state that these are the only two that are affected by\n> this bit in the log message so that readers of \"git log\" do not have\n> to ask the question.\n> \n> The other configuration feature.experimental controls is described\n> in Documentation/config/fetch.txt like this:\n> \n>     fetch.negotiationAlgorithm::\n>             Control how information about the commits in the local repository is\n>             sent when negotiating the contents of the packfile to be sent by the\n>             ...\n>             The default is \"default\" which instructs Git to use the default algorithm\n>             that never skips commits (unless the server has acknowledged it or one\n>             of its descendants). If `feature.experimental` is enabled, then this\n>             setting defaults to \"skipping\".\n>             Unknown values will cause 'git fetch' to error out.\n\nThis was my intention at first: create pointers in both directions:\n\n* The docs for 'feature.experimental' should indicate which options\n  are modified when this config is enabled. Users can then navigate to\n  those options for ideas of what specifically they mean.\n\n* The options for individual config values (such as\n  fetch.negotiationAlgorithm) also mention that their defaults are\n  modified by feature.experimental, so users can understand why the\n  behavior might have changed.\n\n> Alternatively, we can \n> \n>  - Remove the description of other configuration variables affected\n>    by feature.experimental from the description of the experimental\n>    bit in Documentation/config/feature.txt, and replace it with \"The\n>    default values for configuration variables marked with\n>    '[EXPERIMENTAL]' are affected by setting this bit\", and\n\nThis provides a nice way to satisfy the need to know \"what changes\nwhen feature.experimental is enabled?\" without needing to change the\ntext with every update.\n\nThe issue I have is that it's not as simple as things being enabled\nor disabled, because sometimes its a more nuanced change.\n\n>  - Start the description of fetch.negotiationAlgorithm with\n>    [EXPERIMENTAL] like this one does, and remove the sentence about\n>    the experimental bit from there.\n\nI think we still want to indicate something about feature.experimental\nin the affected values. We can establish a clean format for this, such\nas stating the option with [EXPERIMENTAL] but then also saying:\n\n\tIf `feature.experimental` is enabled, then the default changes\n\tfrom `default` to `skipping`.\n\nThe [EXPERIMENTAL] tag provides an easy way to search, but this\nadded description can provide context to someone who navigated to\nthe config without knowing about feature.experimental.\n\nWe should explain these possible values separately from this\nsentence. Here is a possible modification of the entire description\n(format adapted from diff.dirstat):\n\n\nfetch.negotiationAlgorithm::\n\t[EXPERIMENTAL] Control how information about the commits in the local\n\trepository is sent when negotiating the contents of the packfile to be\n\tsent by the server. The following values are supported:\n+\n--\n`default`;;\n\tGit will never skips commits, unless the server has acknowledged it or\n\tone of its descendants.\n`skipping`;;\n\tGit will use an algorithm that skips commits in an effort to converge\n\tfaster,\tbut may result in a larger-than-necessary packfile.\n`noop`;;\n\tDo not send any information at all, which will almost certainly result\n\tin a larger-than-necessary packfile, but will skip the negotiation step.\n--\n+\nIf `feature.experimental` is enabled, then the default value changes from\n`default` to `skipping`. See also the `--negotiation-tip` option for\nlinkgit:git-fetch[1].\n\n\nAnd the docs for `add.interactive.useBuiltin` could be:\n\n\nadd.interactive.useBuiltin::\n\t[EXPERIMENTAL] Enable this value to use the experimental built-in\n\timplementation of the interactive version of linkgit:git-add[1]\n\tinstead of the Perl script version. If `feature.experimental` is\n\tenabled, then the default changes from `false` to `true`.\n\n\nI'm not pushing hard for this direction, but it is one that makes sense\nto me. It allows us to update the individual config values when they are\nadded or removed from the feature.experimental setting without needing\nto also update the feature.experimental setting.\n\nThanks,\n-Stolee\n"},{"id":"427744","messageId":"nycvar.QRO.7.76.6.2106171400360.57@tvgsbejvaqbjf.bet","threadId":"55935","inReplyTo":"20210615164522.1079951-1-tmz@pobox.com","subject":"Re: [PATCH] doc: builtin add -i is enabled by feature.experimental","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-06-17T12:01:27Z","receivedAt":"2021-06-17T12:01:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Todd,\n\n\nOn Tue, 15 Jun 2021, Todd Zullinger wrote:\n\n> Note that add.interactive.useBuiltin is enabled by feature.experimental.\n> It was added in 2df2d81ddd (add -i: use the built-in version when\n> feature.experimental is set, 2020-09-08).\n>\n> Signed-off-by: Todd Zullinger <tmz@pobox.com>\n> ---\n> I was checking my configuration to see if I still needed to have\n> add.interactive.useBuiltin set and noticed that it wasn't listed in the\n> settings enabled by feature.experimental.\n>\n> Unless it's time to take this out of the experimental phase, it seems worth\n> documenting -- if for no other reason than to keep fetch.negotiationAlgorithm\n> from being lonely in the feature.experimental section. ;)\n>\n> FWIW, I've had this enabled for perhaps a year or so and I don't recall any\n> instances where it hasn't worked well.  It has come in handy on some systems\n> where I can't easily install Perl but don't want to give up `add -[ip]`.\n> Thanks Dscho!\n>\n>  Documentation/config/add.txt     | 3 ++-\n>  Documentation/config/feature.txt | 6 ++++++\n>  2 files changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\n> index c9f748f81c..7d6d325571 100644\n> --- a/Documentation/config/add.txt\n> +++ b/Documentation/config/add.txt\n> @@ -9,4 +9,5 @@ add.ignore-errors (deprecated)::\n>  add.interactive.useBuiltin::\n>  \t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n>  \timplementation of the interactive version of linkgit:git-add[1]\n> -\tinstead of the Perl script version. Is `false` by default.\n> +\tinstead of the Perl script version.  If `feature.experimental` is\n\nIt is usually a good idea to stick to the style unless fixing it. In this\ncase, I would suggest to remove the extra space after the period.\n\nOther than that, this patch looks fine to me.\n\nCiao,\nDscho\n\n> +\tenabled, this setting is `true`.  By default, it is `false`.\n> diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt\n> index cdecd04e5b..caaa97dc61 100644\n> --- a/Documentation/config/feature.txt\n> +++ b/Documentation/config/feature.txt\n> @@ -12,6 +12,10 @@ feature.experimental::\n>  \tsetting if you are interested in providing feedback on experimental\n>  \tfeatures. The new default values are:\n>  +\n> +* `add.interactive.useBuiltin=true` which enables the built-in implementation\n> +of the interactive version of linkgit:git-add[1] instead of the Perl script\n> +version.\n> ++\n>  * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by\n>  skipping more commits at a time, reducing the number of round trips.\n>\n>  feature.manyFiles::\n>  \tEnable config options that optimize for repos with many files in the\n> --\n> 2.32.0\n>\n>\n"}]}