Re: [PATCH v2 3/3] t6421: fix test to work when repo dir contains d0
From: Junio C Hamano <hidden>
Date: 2024-08-03 00:27:44
Kyle Lippincott [off-list ref] writes:
quoted
So it really didn't have to clarify what it is looking for, as it would not help seeing what false positives the patterns are designed to avoid matching. Sorry about that.I would have included examples, but they're quite long (>>>80 chars), so seemed very out of place in both commit description and in the codebase.
Absolutely. It turned out not to be so useful to show the shape of
potential matches, like this one:
... run-command.c:733 | d0 | main | child_start |...
To explain why spaces around " d0 " matters, the readers need to
understand that other trace entries that are irrelevant for our
purpose, like this one
... | label:do_write_index /path/to/t/trash directory.../.git/index.lock
we want reject, and for that we want the pattern to be specific
enough by looking for " do " that is followed by " child_start ".
Otherwise the leading paths that can contain anything won't easily
match, and the original of looking for just "d0" was way too error
prone. But it is hard to leave a concise hint for that there.
So, again, sorry about the bad suggestion.
I considered using `awk -F"|" "\$2~/d0/ &&
\$9~/fetch\\.negotiationAlgorithm/{ print }" trace.output >fetches`,
but it was longer, possibly less clear, and less specific (since it
didn't include the $4~/child_start/ condition)Yeah, using the syntactic clue -F"|" would also be a way to convey the intention (i.e. "we are dealing with tabular output and we expect nth column to be X"), but what you have is probably good enough---it certainly is simpler to read and understand. I briefly considered that looking for "| d0 |" (i.e. explicitly mentioning the column separator in the pattern) would make it even more obvious what we are looking for, but having to worry about quoting "|" in regexp would negate the benefit of obviousness out of the approach to use "grep". Thanks.