Re: [PATCH] make slash-rules more readable

["Dr. Adam Nielsen" <admin@in-ici.net>]

On 31.05.19 18:30, Junio C Hamano wrote:
> "Dr. Adam Nielsen" <admin@in-ici.net> writes:
> 
>> gitignore.txt: make slash-rules more readable
>>
>> Remove meta-rule in a paragraph for trailing-slash.
>> Be precise whenever a trailing slash would make a
>> difference. Improve paragraph for pattern without slash.
>> Remove rule for leading slash because its now redundant.
>> Instead, add examples for leading slash and asterix in
>> example section.
>>
>> Signed-off-by: Dr. Adam Nielsen <admin@in-ici.net>
>>
>> ---
>>   Documentation/gitignore.txt | 71 ++++++++++++++++++++++++++-----------
>>   1 file changed, 50 insertions(+), 21 deletions(-)
> 
> I think the updated text is readable, except for one nit.
> 
> Specifically, if you took my suggestion in an earlier review to

I guess you are referencing on your review from 08.05.2019 to which I 
responded On 12.05.19 11:56,

 >> The "note" is not incorrect per-se.  The behaviour described is
 >> because the leading slash is removed for the purpose of textual
 >> matching against paths, but still counts as a non-trailing slash for
 >> the purpose of anchoring the pattern to the level of recursion.
 >>
 >> I am not sure if that is obvious to the readers, though.
 >
 > Yes, its not explained to the reader that the leading slash is removed
 > for the purpose of textual matching. But maybe this is not necessary in
 > order to understand the effect of the pattern.

> explicitly say that leading slash is merely a workaround for a
> string without slash to anchor the pattern to the directory and
> it should be treated as if it does not exist otherwise, then ...
> 
>> + - The pattern `doc/frotz` and `/doc/frotz` have the same effect
>> +   in any `.gitignore` file. Both pattern contain a non-trailing
>> +   slash and thus match relative to the location of the
>> +   `.gitignore` file.
> 
> ... this paragraph wouldn't have been necessary.

I think this above example follows from (and thus isn't necessary, but 
just a fine example)

     + - The pattern is matched relative to the location of
     +   the `.gitignore` file. Except if the pattern contains
     +   no slash [...]

Because a pattern with a leading slash has a slash, it "is matched 
relative to the location of the `.gitignore` file".

> 
> Besides, one extra reason why these two have the same effect is not
> given in the updated text to explain away "To which substring of
> path 'doc/frotz' does that leading slash in /doc/frotz match?"
 >
> The updated text does not seem to explain that the leading slash is
> merely to pretend that the pattern "contains a slash so it does not
> apply in a subdirectory" and for the purpose of pattern matching the
> slash does not participate in the textual match, which seems to have
> been lost in the updated patch, relative to the suggestions raised
> in the review of earlier rounds.

I believe its not said anywhere in the docs that the pattern is compared 
  by a textual match to a piece of the full path of a file\folder (where 
the path is represented as in a unix-like OS).

I feel like your proposal from

On 08.05.19 07:33, Junio C Hamano wrote:
 >  - A leading slash, if any, is implicitly removed before matching the
 >    pattern with the pathname, but the pattern still counts as having
 >    a non-trailing slash for the purpose of the above rule.

is great for everyone who knows about the algorithm in the background, 
but for others it might be unclear what is meant.

For example "pathname" is not explained anywhere. Its not clear if 
"pathname" itself contains a leading slash, or in which format the 
"pathname" is represented, or if its is absolute or relative.
And "implicitly removed before matching.." is maybe a bit confusing for 
people that see the matching algorithm as a black box. If its not 
explained anywhere in detail how the matching algorithm is conducted, 
why would it matter to tell that the leading slash is removed implicitly?

Thats why I think, the case with the leading slash is already covered by 
the paragraph

     + - The pattern is matched relative to the location of
     +   the `.gitignore` file. Except if the pattern contains
     +   no slash [...]

and why I put the further explanations (that are not necessary in my 
opinion, but also not obvious) in the example section.

Thus, I don't feel the need to add another paragraph, but if you want, I 
can add

 >  - A leading slash, if any, is implicitly removed before matching the
 >    pattern with the pathname, but the pattern still counts as having
 >    a non-trailing slash for the purpose of the above rule.

as another bullet to the patch.

All the best,
Adam