Web lists-archives.com

Re: Typo in the .gitignore docs?




Please send plain text mails. HTML mails are dropped by
git@xxxxxxxxxxxxxxx so other people will not see them.

On Sun, Mar 31, 2019 at 6:27 PM Dr. Adam Nielsen <info@xxxxxxxxxxxx> wrote:
>
> Thank you for your quick response.
>
> I will create a pull request and its probably best to continue the discussion from there:

Actually if you want to reach the git development community, this is the place.

> Am So., 31. März 2019 um 11:54 Uhr schrieb Duy Nguyen <pclouds@xxxxxxxxx>:
>>
>> On Sun, Mar 31, 2019 at 2:53 AM Dr. Adam Nielsen <info@xxxxxxxxxxxx> wrote:
>> >
>> > Hi everyone,
>> >
>> > I think there is a typo in the gitignore docs.
>> >
>> > Its stated on https://git-scm.com/docs/gitignore that
>> >
>> > >If the pattern does not contain a slash /, Git treats it as a shell
>> > >glob pattern and checks for a match against the pathname relative to
>> > >the location of the .gitignore file (relative to the toplevel of the
>> > >work tree if not from a .gitignore file).
>> >
>> > I think that maybe two things are not 100% correct. First I think it
>> > should be "If the pattern does contain" instead of "does not contain".
>> > Secondly, I think it should be mentioned that this is only true for
>> > slashes that are not a trailing slash.
>>
>> The trailing slash is already covered in the previous paragraph as you noticed.
>
>
> If this is the purpose of the line  "it is removed for the purpose of the
> following description" then I would highly recommend to remove that line.
> If I am not mistaken, the line is only used for the next paragraph.
> It also makes the paragraph where this line appears over-complicated when one reads it.

I think slashes are mentioned in the next three paragraphs actually.
Although the last one (about leading slash) is probably irrelevant,
because a slash can't be leading and trailing at the same time (and
the whole pattern "/" is probably seen as leading slash).

>> The "does not contain" is correct, but perhaps the wording is a bit
>> too easy to misunderstand. If you go back to the original version of
>> this document in cedb8d5d33 (Create a new manpage for the gitignore
>> format, and reference it elsewhere, 2007-06-02), you can see an
>> example of the "otherwise" part where the present of '/' will anchor
>> the pattern to the current directory
>>
>> +   For example, "Documentation/\*.html" matches
>> +   "Documentation/git.html" but not
>> +   "Documentation/ppc/ppc.html".  A leading slash matches the
>> +   beginning of the pathname; for example, "/*.c" matches
>> +   "cat-file.c" but not "mozilla-sha1/sha1.c".
>>
>> This part was later removed by me in 2e22a85e5c (gitignore.txt:
>> elaborate shell glob syntax, 2018-02-01). In hindsight that was a
>> mistake since having an example seems to help clarify the description.
>
>
> The latter example is still in the documentation. I think that the first
> example is just about the paragraph for the "*". An example for the
> paragraph about the "non-containable" slash "/" would be that
> "Documentation/\*.html" does not match "Foo/Documentation/git.html".
>
>
>>
>>
>> So rewording this paragraph (and keep the 'does not contain' part) to
>> be easier to understand would be great. If you turn 'does not contain'
>> to 'does contain' then you would need to update the 'otherwise'
>> paragraph as well.
>
>
> I do not see any change if you remove the line "Otherwise, Git
> treats the pattern as a shell glob:". The symbols "*", "?", "[]"
> do also work if the pattern does not contain a slash.

These two paragraphs cover the "contain slash" and "not contain slash"
cases, one for each. So if the first paragraph is updated to cover the
'do contain slash' then the second has to cover the other case. You
probably see the problem here, that the current 'otherwise' paragraph
does not really tell us more about the 'do not contain slash' case. I
was suggesting that if we update one, we might want to clarify the
other.

>> I think the part that trips people is the 'pathname' in 'checks for a
>> match against the pathname relative...'. I think the key point is
>> matching the pattern against any pathname _component_ in the path
>> relative to the location of .gitignore. In other words '*.c' would
>> match 'abc.c' component in 'def/abc.c'. 'def/ghi/abc.c' or 'abc.c'.
>
>
> Yes, I absolutely agree, this is the key point that should be made more clear.
>
> I understand with "the location of the .gitignore file (relative to the
> toplevel of the work tree if not from a .gitignore file)" the root
> directory, or a .gitignore file that happens to be in a subdirectory.
> So if we ignore the subdirectory case it would read to me like this :
>
> "If the pattern does not contain a slash , it checks for a match
> against the pathname relative to the root". And according to this
> I would mistakenly think that '*.c' would match '/abc.c' but not
> '/def/abc.c'. If this is not meant, then I do not know why it is
> mentioned in first place that it is matching the pattern "against
> any pathname relative to...". This seems to be always the case.

It's from 81c13fde37 (gitignore.5: Clarify matching rules -
2010-03-05). I think it's more about the 'does contain slash'. I
dunno. Adding Jonathan maybe he remembers something.

>
>
>>
>>
>> The following 'otherwise' paragraph perhaps could also elaborate a bit
>> more, that the pattern is matched against the entire path (relative to
>> .gitignore), not just one path component. The FNM_PATHNAME implies
>> that (because it would not make sense otherwise) but that's just too
>> hard to see.
>
>
> That would be an option. However, since I do not the see purpose of the 'otherwise' case, I would just be clear about it in the mentioned paragraph.
>
>
>>
>>
>> Patches are welcome.
>
>
> I will create a pull request.
>>
>>
>> > You find discussions about this at
>> > https://github.com/git/git-scm.com/issues/1332 and at
>> > https://stackoverflow.com/a/41761521/2311074
>> >
>> > Here is my proposal for an alternative, maybe more clear version:
>> >
>> > >Whenever you have a string that contains a non-trailing slash "/" , its always considered from
>> > >the root. There is no difference between foo/bar and /foo/bar. The pattern foo/ is not
>> > >considered from the root, because it has no non-trailing slash "/".
>> > >One may match a path that does not start at the root by using "**" (see below).
>> >
>> > Also since we are on it, I would suggest to reduce
>> >
>> > >If the pattern ends with a slash, it is removed for the purpose of the following description, but it
>> > >would only find a match with a directory. In other words, foo/ will match a directory foo and
>> > >paths underneath it, but will not match a regular file or a symbolic link foo (this is consistent
>> > >with the way how pathspec works in general in Git).
>> >
>> > to
>> >
>> > >If the pattern ends with a slash, it would only find a match with a directory. In other words, foo/
>> > >will match a directory foo and paths underneath it, but will not match a regular file or a
>> > >symbolic link foo.
>> >
>> > What do you think?
>> >
>> > Best regards,
>> > Adam
>>
>>
>>
>> --
>> Duy
>
>
>> The desscription before cedb8d5d33 (in git-ls-files.txt) uses
>> _filename_ instead of pathname, which is probably clearer:
>>
>> - - if it does not contain a slash '/', it is a shell glob
>> -   pattern and used to match against the filename without
>> -   leading directories.
>
>
> Yes that seems clearer.
>


-- 
Duy