config.txt: add completion for include, includeIf
PATCH 1/1 config.txt: add completion for include, includeIf
From: Manuel Boni <[email protected]>
Git config's tab completion does not yet know about the "include" and "includeIf" sections, nor the related "path" variable.
Add tab completion support for the aforementioned items, along with two new tests, based on the existing ones, specifically for this completion. Variable completion only works for "include" for now.
Credit for the ideas behind this patch goes to
Ævar Arnfjörð Bjarmason.
---
config.txt: add completion for include, includeIf
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1285%2Fziosombrero%2Fcomp-config-include-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1285/ziosombrero/comp-config-include-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1285
Documentation/config/includeif.txt | 6 ++++++ t/t9902-completion.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 Documentation/config/includeif.txt
Documentation/config/includeif.txt new file +6 -0
@@ -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.
t/t9902-completion.sh +13 -0
@@ -2485,6 +2485,13 @@ test_expect_success 'git config - section' ' EOF ' +test_expect_success 'git config - section include, includeIf' ' + test_completion "git config inclu" <<-\EOF + include.Z + includeIf.Z + EOF +' + test_expect_success 'git config - variable name' ' test_completion "git config log.d" <<-\EOF log.date Z
@@ -2493,6 +2500,12 @@ test_expect_success 'git config - variable name' ' EOF ' +test_expect_success 'git config - variable name include' ' + test_completion "git config include.p" <<-\EOF + include.path Z + EOF +' + test_expect_success 'git config - value' ' test_completion "git config color.pager " <<-\EOF false Z
base-commit: 30cc8d0f147546d4dd77bf497f4dec51e7265bd8
Signature · 1 line
gitgitgadget
Documentation/config/includeif.txt line 6
Quoted 12 lines · Manuel Boni via GitGitGadget
Documentation/config/includeif.txt new file +6
-0@@ -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:
Documentation/config.txt +2 -0
@@ -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
A message that is not in the archive (sent off-list or not yet received): [email protected]
[+cc git@vger; please keep the discussion on-list]
Quoted 7 lines · Manuel Boni
Quoted 3 lines
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?
Is there specific syntax that I should use to directly point to those sections? Or should I simply add something such as "Refer to the 'Includes' and 'Conditional Includes' sections"?
Sadly, no, I don't think we can hyperlink to sections easily with asciidoc. I think we will reliably create an "_includes" id for the subsection in the HTML output, but no portable way to create a link. Doing <a href="git-config.html#_includes"> works for HTML, but you'd want to omit that from the man output (and need to structure the text to make sense without the hyperlink).
That's roughly what our linkgit: macro does, but I don't think we have an equivalent for subsections themselves.
So for now, yeah, I think something like:
See the 'Includes' and 'Conditional Includes' headings in the "CONFIGURATION FILE" section of the main linkgit:git-config[1] documentation.
or something similar.
Quoted 11 lines
Quoted 11 lines
Documentation/config.txt +2
-0@@ -445,6 +445,8 @@ include::config/i18n.txt[] include::config/imap.txt[] +include::config/includeif.txt[] + include::config/index.txt[] include::config/init.txt[]
Quoted 5 lines
Quoted 2 lines
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).
Strange, the tests worked fine for me even without that line, but I will add it anyway.
Ah, sorry, I was holding it wrong. I hadn't run "make" to build "config-list.h" when I was testing before. The generate-configlist.sh script just globs Documentation/config/*.txt, so it works as soon as you run "make"[1].
So in that sense your commit message _is_ accurate, because it doesn't affect the documentation at all. But IMHO it is worth including the new entries there.
-Peff
[1] There's an interesting subtlety there. The Makefile rule for config-list.h depends on config/*.txt, too. So it picks up the new file correctly, but it fails to notice if you then switch away from your commit back to vanilla "master". It thinks config-list.h doesn't need to be rebuilt! Probably not worth worrying about too much as we don't often remove files, and certainly not something that needs to be dealt with for your change.
Quoted 9 lines · Manuel Boni via GitGitGadget
From: Manuel Boni <[email protected]>
Git config's tab completion does not yet know about the "include" and "includeIf" sections, nor the related "path" variable.
Add tab completion support for the aforementioned items, along with two new tests, based on the existing ones, specifically for this completion. Variable completion only works for "include" for now.
You may have started this work for tab completion, but I think you should describe it as adding missing documentation (this comment extends to the title of the patch, too). The work to teach "tab completion" about the variables "git config" can take has done long time ago, and the work by this patch to add description for <include> and <includeIf> is used as an input to that previous work, which results in "git config inc<TAB>" to be command line completed.
Quoted 5 lines
Credit for the ideas behind this patch goes to
Ævar Arnfjörð Bjarmason.Signed-off-by Manuel Boni <[email protected]>Based-on-patch-by Ævar Arnfjörð Bjarmason <[email protected]>
The order of these two is backwards, no? Ævar may have written something that inspire you to arrive at this version, which you signed off before sending it out.
Documentation/config/includeif.txt line 3
Quoted 3 lines
Documentation/config/includeif.txt new file +3
-0+include.path:: + +includeIf.<condition>.path::
Losing the extra blank line will make it clearer, if your intention is that the next lines apply to both of the above two.
Documentation/config/includeif.txt line 6
Quoted 3 lines
Documentation/config/includeif.txt new file +3
-0+ Special variables to include other configuration files. See + the "CONFIGURATION FILE" section in the main + linkgit:git-config[1] documentation.
Thanks.
PATCH v2 1/1 config.txt: document include, includeIf
In reply to Manuel Boni via GitGitGadgetFrom: Manuel Boni <[email protected]>
Git config's tab completion does not yet know about the "include" and "includeIf" sections, nor the related "path" variable.
Add a description for these two sections in 'Documentation/config/includeif.txt', which points to git-config's documentation, specifically the "Includes" and "Conditional Includes" subsections.
As a side effect, tab completion can successfully complete the 'include', 'includeIf', and 'include.add' expressions. This effect is tested by two new ad-hoc tests. Variable completion only works for "include" for now.
Credit for the ideas behind this patch goes to
Ævar Arnfjörð Bjarmason.
---
config.txt: add completion for include, includeIf
Update 2022-07-16: I addressed the suggestions by Jeff King and Junio C. Hamano by integrating their fixes and by rewording the commit message, so that emphasis is put on the documentation enhancement and mentioning the improved tab completion as a beneficial side effect.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1285%2Fziosombrero%2Fcomp-config-include-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1285/ziosombrero/comp-config-include-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1285
Range-diff vs v1:
1: 79d877bcd6c ! 1: 38b1860020f config.txt: add completion for include, includeIf
@@ Metadata
Author: Manuel Boni <[email protected]>
## Commit message ##
- config.txt: add completion for include, includeIf
+ config.txt: document include, includeIf
Git config's tab completion does not yet know about the "include" and "includeIf" sections, nor the related "path" variable.
- Add tab completion support for the aforementioned items,
- along with two new tests, based on the existing ones,
- specifically for this completion. Variable completion only works
- for "include" for now.
+ Add a description for these two sections in
+ 'Documentation/config/includeif.txt', which points to git-config's
+ documentation, specifically the "Includes" and "Conditional Includes"
+ subsections.
+
+ As a side effect, tab completion can successfully complete the
+ 'include', 'includeIf', and 'include.add' expressions.
+ This effect is tested by two new ad-hoc tests.
+ Variable completion only works for "include" for now.
Credit for the ideas behind this patch goes to Ævar Arnfjörð Bjarmason.
+ Helped-by: Ævar Arnfjörð Bjarmason <[email protected]>
- Based-on-patch-by: Ævar Arnfjörð Bjarmason <[email protected]>
+
+ ## Documentation/config.txt ##
+@@ Documentation/config.txt: include::config/i18n.txt[] +
+ include::config/imap.txt[]
+
++include::config/includeif.txt[]
++
+ include::config/index.txt[]
+
+ include::config/init.txt[]
## Documentation/config/includeif.txt (new) ##
@@
+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.
++ linkgit:git-config[1] documentation,
++ specifically the "Includes" and "Conditional Includes" subsections.
++
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'git config - section' '
Documentation/config.txt | 2 ++ Documentation/config/includeif.txt | 7 +++++++ t/t9902-completion.sh | 13 +++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 Documentation/config/includeif.txt
Documentation/config.txt +2 -0
@@ -445,6 +445,8 @@ include::config/i18n.txt[] include::config/imap.txt[] +include::config/includeif.txt[] + include::config/index.txt[] include::config/init.txt[]
Documentation/config/includeif.txt new file +7 -0
@@ -0,0 +1,7 @@ +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, + specifically the "Includes" and "Conditional Includes" subsections. +
t/t9902-completion.sh +13 -0
@@ -2485,6 +2485,13 @@ test_expect_success 'git config - section' ' EOF ' +test_expect_success 'git config - section include, includeIf' ' + test_completion "git config inclu" <<-\EOF + include.Z + includeIf.Z + EOF +' + test_expect_success 'git config - variable name' ' test_completion "git config log.d" <<-\EOF log.date Z
@@ -2493,6 +2500,12 @@ test_expect_success 'git config - variable name' ' EOF ' +test_expect_success 'git config - variable name include' ' + test_completion "git config include.p" <<-\EOF + include.path Z + EOF +' + test_expect_success 'git config - value' ' test_completion "git config color.pager " <<-\EOF false Z
base-commit: 9dd64cb4d310986dd7b8ca7fff92f9b61e0bd21a
Signature · 1 line
gitgitgadget
Re: [PATCH v2] config.txt: document include, includeIf
Quoted 4 lines · Manuel Boni via GitGitGadget
Update 2022-07-16: I addressed the suggestions by Jeff King and Junio C. Hamano by integrating their fixes and by rewording the commit message, so that emphasis is put on the documentation enhancement and mentioning the improved tab completion as a beneficial side effect.
Thanks. This version looks OK to me.
-Peff