{"thread":{"id":"58154","subject":"[PATCH] config.txt: add completion for include, includeIf","startedAt":"2022-07-13T19:02:25Z","lastAt":"2022-07-16T22:49:57Z","messageCount":6,"participants":["Manuel Boni via GitGitGadget","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"459002","messageId":"pull.1285.git.1657738937049.gitgitgadget@gmail.com","threadId":"58154","inReplyTo":null,"subject":"[PATCH] config.txt: add completion for include, includeIf","fromName":"Manuel Boni via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-13T19:02:16Z","receivedAt":"2022-07-13T19:02:25Z","isPatch":true,"sender":{"key":"ziosombrero@gmail.com","avatar":null},"body":"From: Manuel Boni \n\nGit config's tab completion does not yet know about the \"include\"\nand \"includeIf\" sections, nor the related \"path\" variable.\n\nAdd tab completion support for the aforementioned items,\nalong with two new tests, based on the existing ones,\nspecifically for this completion. Variable completion only works\nfor \"include\" for now.\n\nCredit for the ideas behind this patch goes to\nÆvar Arnfjörð Bjarmason.\n\nSigned-off-by: Manuel Boni \nBased-on-patch-by: Ævar Arnfjörð Bjarmason \n---\n config.txt: add completion for include, includeIf\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1285%2Fziosombrero%2Fcomp-config-include-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1285/ziosombrero/comp-config-include-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1285\n\n Documentation/config/includeif.txt | 6 ++++++\n t/t9902-completion.sh | 13 +++++++++++++\n 2 files changed, 19 insertions(+)\n create mode 100644 Documentation/config/includeif.txt\n\ndiff --git a/Documentation/config/includeif.txt b/Documentation/config/includeif.txt\nnew file mode 100644\nindex 00000000000..18248cf462e\n--- /dev/null\n+++ b/Documentation/config/includeif.txt\n@@ -0,0 +1,6 @@\n+include.path::\n+\n+includeIf..path::\n+\tSpecial variables to include other configuration files. See\n+\tthe \"CONFIGURATION FILE\" section in the main\n+\tlinkgit:git-config[1] documentation.\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 31526e6b641..43de868b800 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2485,6 +2485,13 @@ test_expect_success 'git config - section' '\n \tEOF\n '\n \n+test_expect_success 'git config - section include, includeIf' '\n+\ttest_completion \"git config inclu\" <<-\\EOF\n+\tinclude.Z\n+\tincludeIf.Z\n+\tEOF\n+'\n+\n test_expect_success 'git config - variable name' '\n \ttest_completion \"git config log.d\" <<-\\EOF\n \tlog.date Z\n@@ -2493,6 +2500,12 @@ test_expect_success 'git config - variable name' '\n \tEOF\n '\n \n+test_expect_success 'git config - variable name include' '\n+\ttest_completion \"git config include.p\" <<-\\EOF\n+\tinclude.path Z\n+\tEOF\n+'\n+\n test_expect_success 'git config - value' '\n \ttest_completion \"git config color.pager \" <<-\\EOF\n \tfalse Z\n\nbase-commit: 30cc8d0f147546d4dd77bf497f4dec51e7265bd8\n-- \ngitgitgadget\n"},{"id":"459004","messageId":"Ys8dwDZK2L4QfSZu@coredump.intra.peff.net","threadId":"58154","inReplyTo":"pull.1285.git.1657738937049.gitgitgadget@gmail.com","subject":"Re: [PATCH] config.txt: add completion for include, includeIf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-13T19:32:16Z","receivedAt":"2022-07-13T19:32:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 13, 2022 at 07:02:16PM +0000, Manuel Boni via GitGitGadget wrote:\n\n> diff --git a/Documentation/config/includeif.txt b/Documentation/config/includeif.txt\n> new file mode 100644\n> index 00000000000..18248cf462e\n> --- /dev/null\n> +++ b/Documentation/config/includeif.txt\n> @@ -0,0 +1,6 @@\n> +include.path::\n> +\n> +includeIf..path::\n> +\tSpecial variables to include other configuration files. See\n> +\tthe \"CONFIGURATION FILE\" section in the main\n> +\tlinkgit:git-config[1] documentation.\n\nHeh, the subject says \"add completion\" but the meat of this patch is\nreally \"add documentation\". I know your ulterior motive is the former,\nbut we should note this affects the latter just as much.\n\nThat said, I think it's an improvement there, too. It helps not only the\ncompletion, but also humans looking in the variable list. And it does\nthe right thing by pointing to the existing documentation, rather than\ntrying to come up with a new description. So I like the direction. I\nhave one fix and one suggestion, though.\n\nFirst the suggestion: rather than just point them to the \"CONFIGURATION\nFILE\" section, there are subsections for \"Includes\" and \"Conditional\nIncludes\". Maybe worth pointing to those specific sections?\n\nAnd the fix: this won't change the documentation unless you also mention\nthe new config/includeif.txt file from the main config.txt, like:\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e376d547ce..5b5b976569 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -445,6 +445,8 @@ include::config/i18n.txt[]\n \n include::config/imap.txt[]\n \n+include::config/includeif.txt[]\n+\n include::config/index.txt[]\n \n include::config/init.txt[]\n\nWithout that, I don't think the completion will find it either (at least\nyour tests did not seem to pass for me until I added it).\n\n-Peff\n"},{"id":"459006","messageId":"xmqqv8s0bxj8.fsf@gitster.g","threadId":"58154","inReplyTo":"pull.1285.git.1657738937049.gitgitgadget@gmail.com","subject":"Re: [PATCH] config.txt: add completion for include, includeIf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-13T19:52:27Z","receivedAt":"2022-07-13T19:52:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Manuel Boni via GitGitGadget\" writes:\n\n> From: Manuel Boni \n>\n> Git config's tab completion does not yet know about the \"include\"\n> and \"includeIf\" sections, nor the related \"path\" variable.\n>\n> Add tab completion support for the aforementioned items,\n> along with two new tests, based on the existing ones,\n> specifically for this completion. Variable completion only works\n> for \"include\" for now.\n\nYou may have started this work for tab completion, but I think you\nshould describe it as adding missing documentation (this comment\nextends to the title of the patch, too). The work to teach \"tab\ncompletion\" about the variables \"git config\" can take has done long\ntime ago, and the work by this patch to add description for\n and is used as an input to that previous work,\nwhich results in \"git config inc\" to be command line completed.\n\n> Credit for the ideas behind this patch goes to\n> Ævar Arnfjörð Bjarmason.\n>\n> Signed-off-by: Manuel Boni \n> Based-on-patch-by: Ævar Arnfjörð Bjarmason \n\nThe order of these two is backwards, no? Ævar may have written\nsomething that inspire you to arrive at this version, which you\nsigned off before sending it out.\n\n> +include.path::\n> +\n> +includeIf..path::\n\nLosing the extra blank line will make it clearer, if your intention\nis that the next lines apply to both of the above two.\n\n> +\tSpecial variables to include other configuration files. See\n> +\tthe \"CONFIGURATION FILE\" section in the main\n> +\tlinkgit:git-config[1] documentation.\n\nThanks.\n"},{"id":"459084","messageId":"YtCG/xVfWKsiSXUp@coredump.intra.peff.net","threadId":"58154","inReplyTo":"d21115d5-8bae-c120-453f-9dbb600d0431@gmail.com","subject":"Re: [PATCH] config.txt: add completion for include, includeIf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-14T21:13:35Z","receivedAt":"2022-07-14T21:13:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[+cc git@vger; please keep the discussion on-list]\n\nOn Thu, Jul 14, 2022 at 09:37:55AM +0200, Manuel Boni wrote:\n\n> > First the suggestion: rather than just point them to the \"CONFIGURATION\n> > FILE\" section, there are subsections for \"Includes\" and \"Conditional\n> > Includes\". Maybe worth pointing to those specific sections?\n> \n> Is there specific syntax that I should use to directly point to those\n> sections? Or should I simply add something such as \"Refer to the 'Includes'\n> and 'Conditional Includes' sections\"?\n\nSadly, no, I don't think we can hyperlink to sections easily with\nasciidoc. I think we will reliably create an \"_includes\" id for the\nsubsection in the HTML output, but no portable way to create a link.\nDoing works for HTML, but you'd\nwant to omit that from the man output (and need to structure the text to\nmake sense without the hyperlink).\n\nThat's roughly what our linkgit: macro does, but I don't think we have\nan equivalent for subsections themselves.\n\nSo for now, yeah, I think something like:\n\n See the 'Includes' and 'Conditional Includes' headings in the\n \"CONFIGURATION FILE\" section of the main linkgit:git-config[1]\n documentation.\n\nor something similar.\n\n> > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > index e376d547ce..5b5b976569 100644\n> > --- a/Documentation/config.txt\n> > +++ b/Documentation/config.txt\n> > @@ -445,6 +445,8 @@ include::config/i18n.txt[]\n> > include::config/imap.txt[]\n> > +include::config/includeif.txt[]\n> > +\n> > include::config/index.txt[]\n> > include::config/init.txt[]\n> > \n> > Without that, I don't think the completion will find it either (at least\n> > your tests did not seem to pass for me until I added it).\n> \n> Strange, the tests worked fine for me even without that line, but I will add\n> it anyway.\n\nAh, sorry, I was holding it wrong. I hadn't run \"make\" to build\n\"config-list.h\" when I was testing before. The generate-configlist.sh\nscript just globs Documentation/config/*.txt, so it works as soon as you\nrun \"make\"[1].\n\nSo in that sense your commit message _is_ accurate, because it doesn't\naffect the documentation at all. But IMHO it is worth including the new\nentries there.\n\n-Peff\n\n[1] There's an interesting subtlety there. The Makefile rule for\n config-list.h depends on config/*.txt, too. So it picks up the new\n file correctly, but it fails to notice if you then switch away from\n your commit back to vanilla \"master\". It thinks config-list.h\n doesn't need to be rebuilt! Probably not worth worrying about too\n much as we don't often remove files, and certainly not something\n that needs to be dealt with for your change.\n"},{"id":"459210","messageId":"pull.1285.v2.git.1658002423864.gitgitgadget@gmail.com","threadId":"58154","inReplyTo":"pull.1285.git.1657738937049.gitgitgadget@gmail.com","subject":"[PATCH v2] config.txt: document include, includeIf","fromName":"Manuel Boni via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-16T20:13:43Z","receivedAt":"2022-07-16T20:13:57Z","isPatch":true,"sender":{"key":"ziosombrero@gmail.com","avatar":null},"body":"From: Manuel Boni \n\nGit config's tab completion does not yet know about the \"include\"\nand \"includeIf\" sections, nor the related \"path\" variable.\n\nAdd a description for these two sections in\n'Documentation/config/includeif.txt', which points to git-config's\ndocumentation, specifically the \"Includes\" and \"Conditional Includes\"\nsubsections.\n\nAs a side effect, tab completion can successfully complete the\n'include', 'includeIf', and 'include.add' expressions.\nThis effect is tested by two new ad-hoc tests.\nVariable completion only works for \"include\" for now.\n\nCredit for the ideas behind this patch goes to\nÆvar Arnfjörð Bjarmason.\n\nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Manuel Boni \n---\n config.txt: add completion for include, includeIf\n \n CC: Ævar Arnfjörð Bjarmason avarab@gmail.com cc: Jeff King peff@peff.net\n \n Update 2022-07-16: I addressed the suggestions by Jeff King and Junio C.\n Hamano by integrating their fixes and by rewording the commit message,\n so that emphasis is put on the documentation enhancement and mentioning\n the improved tab completion as a beneficial side effect.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1285%2Fziosombrero%2Fcomp-config-include-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1285/ziosombrero/comp-config-include-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1285\n\nRange-diff vs v1:\n\n 1: 79d877bcd6c ! 1: 38b1860020f config.txt: add completion for include, includeIf\n @@ Metadata\n Author: Manuel Boni \n \n ## Commit message ##\n - config.txt: add completion for include, includeIf\n + config.txt: document include, includeIf\n \n Git config's tab completion does not yet know about the \"include\"\n and \"includeIf\" sections, nor the related \"path\" variable.\n \n - Add tab completion support for the aforementioned items,\n - along with two new tests, based on the existing ones,\n - specifically for this completion. Variable completion only works\n - for \"include\" for now.\n + Add a description for these two sections in\n + 'Documentation/config/includeif.txt', which points to git-config's\n + documentation, specifically the \"Includes\" and \"Conditional Includes\"\n + subsections.\n +\n + As a side effect, tab completion can successfully complete the\n + 'include', 'includeIf', and 'include.add' expressions.\n + This effect is tested by two new ad-hoc tests.\n + Variable completion only works for \"include\" for now.\n \n Credit for the ideas behind this patch goes to\n Ævar Arnfjörð Bjarmason.\n \n + Helped-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Manuel Boni \n - Based-on-patch-by: Ævar Arnfjörð Bjarmason \n +\n + ## Documentation/config.txt ##\n +@@ Documentation/config.txt: include::config/i18n.txt[]\n + \n + include::config/imap.txt[]\n + \n ++include::config/includeif.txt[]\n ++\n + include::config/index.txt[]\n + \n + include::config/init.txt[]\n \n ## Documentation/config/includeif.txt (new) ##\n @@\n +include.path::\n -+\n +includeIf..path::\n +\tSpecial variables to include other configuration files. See\n +\tthe \"CONFIGURATION FILE\" section in the main\n -+\tlinkgit:git-config[1] documentation.\n ++\tlinkgit:git-config[1] documentation,\n ++\tspecifically the \"Includes\" and \"Conditional Includes\" subsections.\n ++\t\n \n ## t/t9902-completion.sh ##\n @@ t/t9902-completion.sh: test_expect_success 'git config - section' '\n\n\n Documentation/config.txt | 2 ++\n Documentation/config/includeif.txt | 7 +++++++\n t/t9902-completion.sh | 13 +++++++++++++\n 3 files changed, 22 insertions(+)\n create mode 100644 Documentation/config/includeif.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e376d547ce0..5b5b9765699 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -445,6 +445,8 @@ include::config/i18n.txt[]\n \n include::config/imap.txt[]\n \n+include::config/includeif.txt[]\n+\n include::config/index.txt[]\n \n include::config/init.txt[]\ndiff --git a/Documentation/config/includeif.txt b/Documentation/config/includeif.txt\nnew file mode 100644\nindex 00000000000..28b6a52d9f5\n--- /dev/null\n+++ b/Documentation/config/includeif.txt\n@@ -0,0 +1,7 @@\n+include.path::\n+includeIf..path::\n+\tSpecial variables to include other configuration files. See\n+\tthe \"CONFIGURATION FILE\" section in the main\n+\tlinkgit:git-config[1] documentation,\n+\tspecifically the \"Includes\" and \"Conditional Includes\" subsections.\n+\t\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 31526e6b641..43de868b800 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -2485,6 +2485,13 @@ test_expect_success 'git config - section' '\n \tEOF\n '\n \n+test_expect_success 'git config - section include, includeIf' '\n+\ttest_completion \"git config inclu\" <<-\\EOF\n+\tinclude.Z\n+\tincludeIf.Z\n+\tEOF\n+'\n+\n test_expect_success 'git config - variable name' '\n \ttest_completion \"git config log.d\" <<-\\EOF\n \tlog.date Z\n@@ -2493,6 +2500,12 @@ test_expect_success 'git config - variable name' '\n \tEOF\n '\n \n+test_expect_success 'git config - variable name include' '\n+\ttest_completion \"git config include.p\" <<-\\EOF\n+\tinclude.path Z\n+\tEOF\n+'\n+\n test_expect_success 'git config - value' '\n \ttest_completion \"git config color.pager \" <<-\\EOF\n \tfalse Z\n\nbase-commit: 9dd64cb4d310986dd7b8ca7fff92f9b61e0bd21a\n-- \ngitgitgadget\n"},{"id":"459217","messageId":"YtNAi/MDHGLayGY7@coredump.intra.peff.net","threadId":"58154","inReplyTo":"pull.1285.v2.git.1658002423864.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] config.txt: document include, includeIf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-16T22:49:47Z","receivedAt":"2022-07-16T22:49:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 16, 2022 at 08:13:43PM +0000, Manuel Boni via GitGitGadget wrote:\n\n> Update 2022-07-16: I addressed the suggestions by Jeff King and Junio C.\n> Hamano by integrating their fixes and by rewording the commit message,\n> so that emphasis is put on the documentation enhancement and mentioning\n> the improved tab completion as a beneficial side effect.\n\nThanks. This version looks OK to me.\n\n-Peff\n"}]}