Re: [PATCH] config.txt: add completion for include, includeIf
From: Jeff King <hidden>
Date: 2022-07-13 19:32:20
Subsystem:
documentation, the rest · Maintainers:
Jonathan Corbet, Linus Torvalds
On Wed, Jul 13, 2022 at 07:02:16PM +0000, Manuel Boni via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
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