On Thu, May 11, 2017 at 9:42 AM, Jeff King <p...@peff.net> wrote: > On Thu, May 11, 2017 at 09:19:50AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> 1. It says "The included file is expanded immediately, as if its >> contents had been found at the location of the include directive.". At >> first I thought this referred to glob expansion, not >> s/expanded/interpolated/, the example section uses "expand" in the >> context of pathnames, which caused the confusion. > > Perhaps it should say "The contents of the included file are expanded > immediately, as if they had been found at..."? > >> Maybe this would make that less confusing by saying the same thing >> without using the same phrasing to mean completely different things: >> >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index 475e874d51..49855353c7 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -167,8 +167,8 @@ Example >> >> [include] >> path = /path/to/foo.inc ; include by absolute path >> - path = foo ; expand "foo" relative to the current file >> - path = ~/foo ; expand "foo" in your `$HOME` directory >> + path = foo ; find & include "foo" relative to the current >> file >> + path = ~/foo ; find & include "foo" in your `$HOME` directory > > Yeah, probably makes sense to use a different word than "expand" here. > >> 2. There is no example of such expansion for IncludeIf, prose should >> always be backed up by examples for the lazy reader when possible: >> >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index 475e874d51..fc4b87cd7e 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -173,6 +173,8 @@ Example >> ; include if $GIT_DIR is /path/to/foo/.git >> [includeIf "gitdir:/path/to/foo/.git"] >> path = /path/to/foo.inc >> + path = foo ; find & include "foo" relative to the current >> file >> + path = ~/foo ; find & include "foo" in your `$HOME` directory >> >> ; include for all repositories inside /path/to/group >> [includeIf "gitdir:/path/to/group/"] > > Yeah, makes sense. > >> 3. When I read reference docs I almost never start at the beginning & >> read it all the way through, in this case I thought I could help >> someone out by maybe answering a question on the ML quickly, so I went >> to the examples section, found no example (fixed above), then searched >> for "relative" or "path" in my pager and the only results were for the >> "Includes" section that has a paragraph that's only talking about >> "include.path". > > I do, too. And I'm all in favor of structuring things to accommodate > that flow. But at some point we have to assume the user actually reads > the documentation. :) > >> Of course we say "same rules apply" below, but I managed not to spot >> that. Maybe this: >> >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index 475e874d51..b35d9a7b80 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -95,8 +95,11 @@ Conditional includes >> >> You can include a config file from another conditionally by setting a >> `includeIf.<condition>.path` variable to the name of the file to be >> -included. The variable's value is treated the same way as >> -`include.path`. `includeIf.<condition>.path` can be given multiple times. >> +included. >> + >> +If variable's value is a relative path it's treated the same way as >> +`include.path`. `includeIf.<condition>.path` can also be given >> +multiple times. > > I don't like this because it copies the rules for _one_ property to the > conditional section. What happens when you're looking for some other > property of include.path?
Yeah, as I said once I wrote it up I found it wasn't really any better, but just wanted to send an explanation for why I didn't find it while I remembered, as a sort of case study. > I think in your case you found the "relative" section in the earlier > paragraph, but there was no link there to the includeIf behavior. So > should that earlier paragraph just get the "includeIf.path behaves the > same way" mention? > > I suspect that whole paragraph under Includes could be reworded to make > it clear that anything it is saying applies equally to include.$key and > includeIf.*.$key, and then that would future-proof us for other > modifications.
