Re: [PATCH] config.txt: add completion for include, includeIf
- From
Jeff King <peff@peff.net>
- Date
- Jul 13, 2022, 19:32 UTC
- Message-ID
- <Ys8dwDZK2L4QfSZu@coredump.intra.peff.net>
- In-Reply-To
- <pull.1285.git.1657738937049.gitgitgadget@gmail.com>
On Wed, Jul 13, 2022 at 07:02:16PM +0000, Manuel Boni via GitGitGadget wrote:
Show 12 quoted lines
> diff --git a/Documentation/config/includeif.txt b/Documentation/config/includeif.txt > new file mode 100644 > index 00000000000..18248cf462e > --- /dev/null > +++ b/Documentation/config/includeif.txt > @@ -0,0 +1,6 @@ > +include.path:: > + > +includeIf.<condition>.path:: > + Special variables to include other configuration files. See > + the "CONFIGURATION FILE" section in the main > + linkgit:git-config[1] documentation.
Heh, the subject says "add completion" but the meat of this patch is really "add documentation". I know your ulterior motive is the former, but we should note this affects the latter just as much.
That said, I think it's an improvement there, too. It helps not only the completion, but also humans looking in the variable list. And it does the right thing by pointing to the existing documentation, rather than trying to come up with a new description. So I like the direction. I have one fix and one suggestion, though.
First the suggestion: rather than just point them to the "CONFIGURATION FILE" section, there are subsections for "Includes" and "Conditional Includes". Maybe worth pointing to those specific sections?
And the fix: this won't change the documentation unless you also mention the new config/includeif.txt file from the main config.txt, like:
diff --git a/Documentation/config.txt b/Documentation/config.txt index e376d547ce..5b5b976569 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -445,6 +445,8 @@ include::config/i18n.txt[] include::config/imap.txt[] +include::config/includeif.txt[] + include::config/index.txt[] include::config/init.txt[] Without that, I don't think the completion will find it either (at least your tests did not seem to pass for me until I added it). -Peff