Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> On Sun, 14 Aug 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>>
>> > - test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g')
>> > + test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g' -e 'y/>/_/')
>> ...
> I know that this is so because my first iteration of the patch did exactly
> what you suggested.
You probably should have stepped back and taken a deep breath before
writing the second round. Doing so after writing it before sending
would also have been OK.
Among three characters that are special cased here, the problem "if
we squish a run of problematic characters into one underscore, we
risk making the result ambiguous" is NOT limited to '>'; it is not a
new problem with '>', either. I can think of two other possible
solutions offhand:
(1) drop "squishing a run", i.e. [/ ]*, from the regexp, and rename
existing test vectors that would be affected;
(2) change the string used in the offending test so that squishing
will not make the result ambiguous.
before special casing "y/>/_/"; as there is nothing in ">" that
inherently causes the ambiguity that won't be caused by "/" or " ",
adding second clause to the sed expression that does things
differently only for ">" is simply wrong. After all, you only
wanted to affect the "prefix=-->" test and not the whole set of the
tests in the script.
Obviously (1) is a lot of impact with little gain, and as Jacob
already offered to do, I think (2) is a lot more sensible solution
and it also is more in line with your "If it isn't broken, do not
fix it", I would say.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html