{"thread":{"id":"48105","subject":"[PATCH] doc: clarify non-recursion for ignore paths like `foo/`","startedAt":"2018-03-21T11:13:43Z","lastAt":"2018-03-21T12:46:18Z","messageCount":6,"participants":["Dakota Hawkins","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"342400","messageId":"CAHnyXxSqtB=bSbA83V6HC6-aPCxw60h1iKQaa6ChwsmcUUCd0w@mail.gmail.com","threadId":"48105","inReplyTo":null,"subject":"[PATCH] doc: clarify non-recursion for ignore paths like `foo/`","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T11:13:37Z","receivedAt":"2018-03-21T11:13:43Z","isPatch":true,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":">> At any rate, would it at least be a good idea to make the \"trailing\n>> slash halts recursion, won't consider nested .gitignore files\"\n>> explicit in the `.gitignore` doc? Unless I'm missing it, I don't think\n>> that behavior is called out (or at least not called out concisely/in\n>> one place). It looks like this is all there is:\n>\n> Yeah, it's definitely come up multiple times over the years. I don't\n> know what all is in gitignore(5), but if it's not mentioned it probably\n> should be.\n>\n>>     \"If the pattern ends with a slash, it is removed for the purpose\n>> of the following description, but it would only find a match with a\n>> directory. In other words, foo/ will match a directory foo and paths\n>> underneath it, but will not match a regular file or a symbolic link\n>> foo (this is consistent with the way how pathspec works in general in\n>> Git).\"\n>>\n>> Also, I'm not sure what the \"following description\" is in \"it is\n>> removed for the purpose of the following description\". Is that trying\n>> to imply \"excluded from the rest of the doc\"?\n>\n> I think it means \"for the rest of the description of how the patterns\n> work\". I.e., \"foo/\" matches as \"foo\" when the rest of the matching rules\n> are applied. I agree it's a bit awkward. Patches welcome. :)\n\nI hope this is correct. I tried to follow the instructions. Please let\nme know if I need to fix something with how I'm sending this!\n\n-- >8 --\nSubject: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`\n\nThe behavior of .gitignore patterns ending with trailing slashes is\nunclear. The user may expect subsequent matching patterns to matter, while\nthey do not. For example:\n\n  foo/       # Ignores `foo` directories and everything inside of them\n  !foo/*.txt # Does nothing\n\nExplain this behavior (and its implications) more explicitly.\n\nSigned-off-by: Dakota Hawkins <daktoahawkins@gmail.com>\n---\n Documentation/gitignore.txt | 17 +++++++++++------\n 1 file changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\nindex ff5d7f9ed..e9c34c1d5 100644\n--- a/Documentation/gitignore.txt\n+++ b/Documentation/gitignore.txt\n@@ -89,12 +89,17 @@ PATTERN FORMAT\n    Put a backslash (\"`\\`\") in front of the first \"`!`\" for patterns\n    that begin with a literal \"`!`\", for example, \"`\\!important!.txt`\".\n\n- - If the pattern ends with a slash, it is removed for the\n-   purpose of the following description, but it would only find\n-   a match with a directory.  In other words, `foo/` will match a\n-   directory `foo` and paths underneath it, but will not match a\n-   regular file or a symbolic link `foo` (this is consistent\n-   with the way how pathspec works in general in Git).\n+ - If the pattern ends with a slash it will match directories but prevent\n+   further recursion into subdirectories. In other words, `foo/` will match a\n+   directory `foo`, excluding files and paths underneath it, but will not match\n+   a regular file or a symbolic link `foo` (this is consistent with the way\n+   that pathspec works in general in Git). Consequently, `foo/` will prevent\n+   consideration of subsequent matches, including exclusions (for example,\n+   `!foo/*.noignore`). In order to match `foo/` directories while allowing for\n+   possible later exclusions, consider using a trailing wildcard (`foo/*`).\n+   Note that matching directories with a trailing wildcard incurs some\n+   additional performance cost, since it requires recursion into\n+   subdirectories.\n\n  - If the pattern does not contain a slash '/', Git treats it as\n    a shell glob pattern and checks for a match against the\n-- \n2.16.2.windows.1\n"},{"id":"342401","messageId":"87muz1brgt.fsf@evledraar.gmail.com","threadId":"48105","inReplyTo":"CAHnyXxSqtB=bSbA83V6HC6-aPCxw60h1iKQaa6ChwsmcUUCd0w@mail.gmail.com","subject":"Re: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-21T11:25:38Z","receivedAt":"2018-03-21T11:25:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 21 2018, Dakota Hawkins jotted:\n\n>>> At any rate, would it at least be a good idea to make the \"trailing\n>>> slash halts recursion, won't consider nested .gitignore files\"\n>>> explicit in the `.gitignore` doc? Unless I'm missing it, I don't think\n>>> that behavior is called out (or at least not called out concisely/in\n>>> one place). It looks like this is all there is:\n>>\n>> Yeah, it's definitely come up multiple times over the years. I don't\n>> know what all is in gitignore(5), but if it's not mentioned it probably\n>> should be.\n>>\n>>>     \"If the pattern ends with a slash, it is removed for the purpose\n>>> of the following description, but it would only find a match with a\n>>> directory. In other words, foo/ will match a directory foo and paths\n>>> underneath it, but will not match a regular file or a symbolic link\n>>> foo (this is consistent with the way how pathspec works in general in\n>>> Git).\"\n>>>\n>>> Also, I'm not sure what the \"following description\" is in \"it is\n>>> removed for the purpose of the following description\". Is that trying\n>>> to imply \"excluded from the rest of the doc\"?\n>>\n>> I think it means \"for the rest of the description of how the patterns\n>> work\". I.e., \"foo/\" matches as \"foo\" when the rest of the matching rules\n>> are applied. I agree it's a bit awkward. Patches welcome. :)\n>\n> I hope this is correct. I tried to follow the instructions. Please let\n> me know if I need to fix something with how I'm sending this!\n>\n> -- >8 --\n> Subject: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`\n>\n> The behavior of .gitignore patterns ending with trailing slashes is\n> unclear. The user may expect subsequent matching patterns to matter, while\n> they do not. For example:\n>\n>   foo/       # Ignores `foo` directories and everything inside of them\n>   !foo/*.txt # Does nothing\n>\n> Explain this behavior (and its implications) more explicitly.\n>\n> Signed-off-by: Dakota Hawkins <daktoahawkins@gmail.com>\n> ---\n>  Documentation/gitignore.txt | 17 +++++++++++------\n>  1 file changed, 11 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\n> index ff5d7f9ed..e9c34c1d5 100644\n> --- a/Documentation/gitignore.txt\n> +++ b/Documentation/gitignore.txt\n> @@ -89,12 +89,17 @@ PATTERN FORMAT\n>     Put a backslash (\"`\\`\") in front of the first \"`!`\" for patterns\n>     that begin with a literal \"`!`\", for example, \"`\\!important!.txt`\".\n>\n> - - If the pattern ends with a slash, it is removed for the\n> -   purpose of the following description, but it would only find\n> -   a match with a directory.  In other words, `foo/` will match a\n> -   directory `foo` and paths underneath it, but will not match a\n> -   regular file or a symbolic link `foo` (this is consistent\n> -   with the way how pathspec works in general in Git).\n> + - If the pattern ends with a slash it will match directories but prevent\n> +   further recursion into subdirectories. In other words, `foo/` will match a\n> +   directory `foo`, excluding files and paths underneath it, but will not match\n> +   a regular file or a symbolic link `foo` (this is consistent with the way\n> +   that pathspec works in general in Git). Consequently, `foo/` will prevent\n> +   consideration of subsequent matches, including exclusions (for example,\n> +   `!foo/*.noignore`). In order to match `foo/` directories while allowing for\n> +   possible later exclusions, consider using a trailing wildcard (`foo/*`).\n> +   Note that matching directories with a trailing wildcard incurs some\n> +   additional performance cost, since it requires recursion into\n> +   subdirectories.\n>\n>   - If the pattern does not contain a slash '/', Git treats it as\n>     a shell glob pattern and checks for a match against the\n\nJust before the context your patch quotes, we have this in gitignore(5)\nalready:\n\n    [...]It is not possible to re-include a file if a parent directory\n    of that file is excluded. Git doesn’t list excluded directories for\n    performance reasons, so any patterns on contained files have no\n    effect, no matter where they are defined.[...]\n\nI can't see how your change to the documentation doesn't just re-state\nwhat we already have documented, which is not to say the docs can't be\nimproved, but then we should clearly state this in one place, not\nre-state it within the span of a few paragraphs.\n"},{"id":"342402","messageId":"CAHnyXxQRfzAT75P7dxvdEEN_FRYCc7j97VEfL8tTUgVdeK-O+g@mail.gmail.com","threadId":"48105","inReplyTo":"87muz1brgt.fsf@evledraar.gmail.com","subject":"Re: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T11:53:24Z","receivedAt":"2018-03-21T11:53:31Z","isPatch":true,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"First, apologies since I didn't get the in-reply-to correct in my\nemail header. I'm not sure how to do that (using gmail/gsuite).\n\nMeant to reply to:\nhttps://public-inbox.org/git/20180321075041.GA24701@sigill.intra.peff.net/\n\n> Just before the context your patch quotes, we have this in gitignore(5)\n> already:\n>\n>     [...]It is not possible to re-include a file if a parent directory\n>     of that file is excluded. Git doesn’t list excluded directories for\n>     performance reasons, so any patterns on contained files have no\n>     effect, no matter where they are defined.[...]\n>\n> I can't see how your change to the documentation doesn't just re-state\n> what we already have documented, which is not to say the docs can't be\n> improved, but then we should clearly state this in one place, not\n> re-state it within the span of a few paragraphs.\n\nContext:\n\nThis came up originally because of confusion with .gitattributes\npatterns, since gitattributes doesn't have the same `foo/` matching\nbehavior. Jeff King was kind enough to prepare a patch for that\ndocumentation here:\nhttps://public-inbox.org/git/20180320041454.GA15213@sigill.intra.peff.net/\n\nRe-reading the section you quoted again a couple of times you're\ncorrect, but somehow that wasn't clear to me despite reading/searching\nfor what I wanted to see several times.\n\nWhile what I wrote may need improvement, I think there are a couple of\nvalid concerns with the way this behavior is documented currently:\n\n  - Generally: Reading about pattern matching for .gitignore is\nawkward on its face, since positive matches have negative consequences\n(in other words `include = !match`).\n  - Specifically: This behavior that is specific to `foo/` matches is\ndocumented in the section for `!foo` matches. If you're trying to find\nthe implications of `foo/` you may not have read about `!foo` as\ncarefully.\n\nSince this behavior is practically applicable to both pattern formats,\nand since patterns in the sum of a repo's .gitignore files can get\nsomewhat complicated, I think it would be a good idea to either:\n\n  - Do this and basically explain the same behavior twice in two\npattern format sections, or\n  - Pull the documentation for this behavior out into another section\nwhere users would be likely to find and understand it if they're\nlooking into either pattern format\n\nDoes that make sense?\n\nWhat do you think?\n\nThanks for the feedback,\n\n- Dakota\n"},{"id":"342403","messageId":"CAHnyXxQ6QH4St0D0u8BrW7i1=r2cgUjwDhRa4e98fmO1tksp-Q@mail.gmail.com","threadId":"48105","inReplyTo":"CAHnyXxQRfzAT75P7dxvdEEN_FRYCc7j97VEfL8tTUgVdeK-O+g@mail.gmail.com","subject":"Re: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T12:13:45Z","receivedAt":"2018-03-21T12:13:52Z","isPatch":true,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"One additional note, I think I was was wrong about this:\n\n  \"In order to match `foo/` directories while allowing for\npossible later exclusions, consider using a trailing wildcard (`foo/*`).\nNote that matching directories with a trailing wildcard incurs some\nadditional performance cost, since it requires recursion into\nsubdirectories.\"\n\n`foo/*` will let `!foo/file` work but still seems to prevent `!foo/bar/file`.\n\nOn Wed, Mar 21, 2018 at 7:53 AM, Dakota Hawkins\n<dakota@dakotahawkins.com> wrote:\n> First, apologies since I didn't get the in-reply-to correct in my\n> email header. I'm not sure how to do that (using gmail/gsuite).\n>\n> Meant to reply to:\n> https://public-inbox.org/git/20180321075041.GA24701@sigill.intra.peff.net/\n>\n>> Just before the context your patch quotes, we have this in gitignore(5)\n>> already:\n>>\n>>     [...]It is not possible to re-include a file if a parent directory\n>>     of that file is excluded. Git doesn’t list excluded directories for\n>>     performance reasons, so any patterns on contained files have no\n>>     effect, no matter where they are defined.[...]\n>>\n>> I can't see how your change to the documentation doesn't just re-state\n>> what we already have documented, which is not to say the docs can't be\n>> improved, but then we should clearly state this in one place, not\n>> re-state it within the span of a few paragraphs.\n>\n> Context:\n>\n> This came up originally because of confusion with .gitattributes\n> patterns, since gitattributes doesn't have the same `foo/` matching\n> behavior. Jeff King was kind enough to prepare a patch for that\n> documentation here:\n> https://public-inbox.org/git/20180320041454.GA15213@sigill.intra.peff.net/\n>\n> Re-reading the section you quoted again a couple of times you're\n> correct, but somehow that wasn't clear to me despite reading/searching\n> for what I wanted to see several times.\n>\n> While what I wrote may need improvement, I think there are a couple of\n> valid concerns with the way this behavior is documented currently:\n>\n>   - Generally: Reading about pattern matching for .gitignore is\n> awkward on its face, since positive matches have negative consequences\n> (in other words `include = !match`).\n>   - Specifically: This behavior that is specific to `foo/` matches is\n> documented in the section for `!foo` matches. If you're trying to find\n> the implications of `foo/` you may not have read about `!foo` as\n> carefully.\n>\n> Since this behavior is practically applicable to both pattern formats,\n> and since patterns in the sum of a repo's .gitignore files can get\n> somewhat complicated, I think it would be a good idea to either:\n>\n>   - Do this and basically explain the same behavior twice in two\n> pattern format sections, or\n>   - Pull the documentation for this behavior out into another section\n> where users would be likely to find and understand it if they're\n> looking into either pattern format\n>\n> Does that make sense?\n>\n> What do you think?\n>\n> Thanks for the feedback,\n>\n> - Dakota\n"},{"id":"342404","messageId":"87woy5y5y8.fsf@evledraar.gmail.com","threadId":"48105","inReplyTo":"CAHnyXxQRfzAT75P7dxvdEEN_FRYCc7j97VEfL8tTUgVdeK-O+g@mail.gmail.com","subject":"Re: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-21T12:21:51Z","receivedAt":"2018-03-21T12:21:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 21 2018, Dakota Hawkins wrote:\n\n> Re-reading the section you quoted again a couple of times you're\n> correct, but somehow that wasn't clear to me despite reading/searching\n> for what I wanted to see several times.\n\nFWIW I just knew this because I'd run into this the other day, so it was\nfresh in my mind.\n\n> While what I wrote may need improvement, I think there are a couple of\n> valid concerns with the way this behavior is documented currently:\n>\n>   - Generally: Reading about pattern matching for .gitignore is\n> awkward on its face, since positive matches have negative consequences\n> (in other words `include = !match`).\n>   - Specifically: This behavior that is specific to `foo/` matches is\n> documented in the section for `!foo` matches. If you're trying to find\n> the implications of `foo/` you may not have read about `!foo` as\n> carefully.\n>\n> Since this behavior is practically applicable to both pattern formats,\n> and since patterns in the sum of a repo's .gitignore files can get\n> somewhat complicated, I think it would be a good idea to either:\n\nI think it makes more sense to document it where it's documented now,\ni.e. under how \"!\" works in general, since this is an edge case with\nnegative matching.\n\nWhether you specify \"foo\" or \"foo/\" is then just an unrelated edge case\nin how we match directories v.s. files, but doesn't per-se have anything\nto do with how the inversion rules work, so I think it makes sense to\ndocument it where we document the inversion rules.\n\nI.e. you'd get the same for all of (assuming a directory \"foo\"):\n\n    f*\n    !*.txt\n\n    foo\n    !*.txt\n\n    foo/\n    !*.txt\n\nSo what we subsequently exclude just because it's deeper in the tree has\nnothing to do with how the disambiguation syntax for matching the\ndirectory looks like.\n\n>   - Do this and basically explain the same behavior twice in two\n> pattern format sections, or\n>   - Pull the documentation for this behavior out into another section\n> where users would be likely to find and understand it if they're\n> looking into either pattern format\n\nI think it can certainly be made clearer, but maybe with a practical\nexample (per above) where we also show the dir structure that\nwould/wouldn't be matched.\n\nI just chimed in on this one because your patch says the docs are\n\"unclear\" then \"Explain this behavior (and its implications) more\nexplicitly\" without a reference to the existing adjacent docs. I think\nwhatever we do we should be unifying these docs if they're amended, not\nstating this twice.\n"},{"id":"342406","messageId":"CAHnyXxRYjCey8N82TfbMbv1+tXNDN4df_UWZ7znaYUj4wcS5HA@mail.gmail.com","threadId":"48105","inReplyTo":"87woy5y5y8.fsf@evledraar.gmail.com","subject":"Re: [PATCH] doc: clarify non-recursion for ignore paths like `foo/`","fromName":"Dakota Hawkins","fromEmail":"dakota@dakotahawkins.com","sentAt":"2018-03-21T12:46:12Z","receivedAt":"2018-03-21T12:46:18Z","isPatch":true,"sender":{"key":"dakota@dakotahawkins.com","avatar":null},"body":"> I think it makes more sense to document it where it's documented now,\n> i.e. under how \"!\" works in general, since this is an edge case with\n> negative matching.\n\nMy reasoning is/was that while yes, that's true, I think it's likely\nthat the positive matches would be added before (in terms of git\nhistory) the exclusions. In other words it may not occur to the user\nwriting an early version of `.gitignore` to think about exclusions if\nthey don't need to exclude anything yet -- the eventual symptoms of\nthis problem may be \"far away\" from the cause, making it harder to\ndiagnose (I'm living proof of that!)\n\n> I think it can certainly be made clearer, but maybe with a practical\n> example (per above) where we also show the dir structure that\n> would/wouldn't be matched.\n>\n> I just chimed in on this one because your patch says the docs are\n> \"unclear\" then \"Explain this behavior (and its implications) more\n> explicitly\" without a reference to the existing adjacent docs. I think\n> whatever we do we should be unifying these docs if they're amended, not\n> stating this twice.\n\nI think my inability to see it despite knowing exactly what I was\nlooking for (this time) makes it \"unclear\", at least to dummies me, so\nI feel like that statement is at least somewhat defensible :)\n\nIf the explanation were unified, would the idiomatic way to do that be\nto add some section way down the document with a couple of `(see SOME\nDETAILS ABOUT THIS below)` and some examples in the examples section?\n"}]}