Jari Aalto <jari.aalto@xxxxxxxxx> writes:
> Add end anchor to limit regexp '[<>=]){7}'; the size of the conflict
> markers. This prevents in some extent incorrect matches of underlined
> headings:
I thought modern file-level merge added comments on where things
came from on <<< and >>> lines. Insisting on /^={7}$/ might be
Ok, but are you sure about <<< and >>> lines?
> Other changes:
I would usually say "Don't. If you want to have clean-up
patches, do them as separate patches, please", here. This is
especially true if your primary point of the patch is sound but
"Other changes" are questionable --- you do not want a good fix
rejected because of extra that came with it that are bad.
> - Collapse multiple Perl warn functions calls into onex
Ok, but I can see you haven't even tested this patch with one of
the warn that supposedly has three strings as parameters, but
have semicolon between the second and the third.
> - Add empty lines to separate code chunk paragraphs
Ok.
> - Add prototype to function definition: sub bad_line ($,$)
Why? I hate when people use Perl5 line-noise prototypes,
perhaps only because they look cool. You are getting $why and
$line that are always scalar anyway, and do not even benefit
from any magic coercing behaviour the line-noise prototype gives
you.
> - Change 'print STDERR' calls to standard warn()
Ok.
Won't apply as is, and will wait for a bit more discussion
hoping somebody comes up with a bit more clever and robust
solution for the main point. Things to consider (and "things we
have considered when we did it this way first").
- "^<{7}" / ">{7}" can be followed by " this came from this branch";
- "^={7}$" is probably safer than the current one but not
perfect;
- we could insist that we see a line with /^<{7}/ before
seeing such a line to declare it leftover, but that is not
useful as the user can resolve <<< part while forgetting ===
marker;
-
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
