git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: "Dr. Adam Nielsen" <admin@in-ici.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] make slash-rules more readable
Date: Fri, 31 May 2019 19:24:25 +0200
Message-ID: <88c6cf45-3908-6561-c3bf-11adf628f8af@in-ici.net> (raw)
In-Reply-To: <xmqqh89awprl.fsf@gitster-ct.c.googlers.com>


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

  reply	other threads:[~2019-05-31 17:24 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31  7:44 Dr. Adam Nielsen
2019-05-31 16:30 ` Junio C Hamano
2019-05-31 17:24   ` Dr. Adam Nielsen [this message]
2019-05-31 17:40     ` Junio C Hamano
2019-06-01  9:33   ` Philip Oakley
2019-06-02  9:01     ` Dr. Adam Nielsen
2019-06-03 18:01     ` Junio C Hamano
2019-06-04 10:40       ` Philip Oakley
2019-06-01  9:23 ` Philip Oakley
2019-06-04 12:34 ` Philip Oakley
2019-06-04 17:22   ` Dr. Adam Nielsen
  -- strict thread matches above, loose matches on Subject: below --
2020-02-15  3:08 Pavel Ivashkov
2019-06-04 17:34 Dr. Adam Nielsen
2019-06-25 11:05 ` Dr. Adam Nielsen
2019-06-25 11:31   ` Philip Oakley
2019-06-27 17:10     ` Dr. Adam Nielsen
2019-07-04 10:40       ` Philip Oakley
2019-07-04 10:46         ` Philip Oakley
2019-06-27 17:43   ` Junio C Hamano
2019-06-02  9:04 Dr. Adam Nielsen
2019-05-31 18:17 Dr. Adam Nielsen
2019-05-31 18:16 Dr. Adam Nielsen
2019-05-18 14:13 Dr. Adam Nielsen
2019-05-19  1:59 ` Junio C Hamano
2019-05-19  6:59 ` Johannes Sixt
2019-05-18 14:07 Dr. Adam Nielsen
2019-05-18 19:34 ` Philip Oakley
2019-05-19 15:33   ` Dr. Adam Nielsen
     [not found]     ` <0c2894ce-7eab-8207-9af8-7ce5e779d4ec@iee.org>
2019-05-29  8:28       ` Dr. Adam Nielsen
2019-05-07 10:45 Dr. Adam Nielsen
2019-05-08  5:33 ` Junio C Hamano
2019-05-12  9:56   ` Dr. Adam Nielsen
2019-05-17 21:43     ` Dr. Adam Nielsen
2019-05-18  6:42       ` Johannes Sixt
2019-05-18 13:20         ` Dr. Adam Nielsen
2019-04-26 14:32 Dr. Adam Nielsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=88c6cf45-3908-6561-c3bf-11adf628f8af@in-ici.net \
    --to=admin@in-ici.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git