From: "Dr. Adam Nielsen" <firstname.lastname@example.org> To: Junio C Hamano <email@example.com> Cc: firstname.lastname@example.org Subject: Re: [PATCH] make slash-rules more readable Date: Fri, 31 May 2019 19:24:25 +0200 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> On 31.05.19 18:30, Junio C Hamano wrote: > "Dr. Adam Nielsen" <email@example.com> 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 <firstname.lastname@example.org> >> >> --- >> 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
next prev parent 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] ` <email@example.com> 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] make slash-rules more readable' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).