Re: [PATCH] pre-commit hook: less easily-tripped conflict marker detection
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:31
Eric Wong [off-list ref] writes:
This should make adding asciidoc files to Documentation easier. Only complain about conflict markers if we see that we have some combination of '<<<<<<< ', '>>>>>>> ', and '======='.
Are you sure everybody uses exactly seven? I did not have SP after a run of '<' and '>' because I didn't know.
Also add a NO_VERIFY environment check to this hook, in case there's something that we want to force in but still gets tripped by this hook.
Hmm. Undecided.
quoted hunk
@@ -24,8 +25,9 @@ perl -e '... + my $in_unresolved;
Unused as far as I can see.
+ sub show_unresolved {
+ # if we want even less easily-tripped checks,
+ # change the "||" to "&&" here. Right now, we can deal with
+ # the case where somebody removed one of the <{7} or >{7} lines
+ # but left the other one (as well as ={7}) in there.I think '||' is fine as is -- I think <<< and === removed with
quoted
quoted
left is a common mistake (think of superseding a smallish
"our change" between <<< and === with much larger and polished upstream "their change" after ===). But I am not sure about the code around here:
+ if (($unresolved[0]->[0] =~ /^<{7} / ||
+ $unresolved[-1]->[0] =~ /^>{7} /) &&
+ grep { $_->[0] =~ /^={7}$/ } @unresolved) {
+ bad_common();
+ foreach my $l (@unresolved) {
+ print STDERR "* unresolved merge conflict (line $l->[1])\n";
+ print STDERR "$filename:$l->[1]:$l->[0]\n"
+ }
+ }- why check only the first and last element in @unresolved but use all of them without checking the ones in-between? - if you are keeping the range in an array, maybe the error message can point at the range. Printing $l->[0] is not so useful (the user sees only the conflict marker) but the original code did so only because it operated on one-line at a time.
quoted hunk
+ @unresolved = (); + } + while (<>) { if (m|^diff --git a/(.*) b/\1$|) { $filename = $1; + show_unresolved() if @unresolved; next; } if (/^@@ -\S+ \+(\d+)/) {@@ -61,8 +85,8 @@ perl -e ' if (/^\s* /) { bad_line("indent SP followed by a TAB", $_); } - if (/^(?:[<>=]){7}/) { - bad_line("unresolved merge conflict", $_); + if (/^[<>]{7} / || /^={7}$/) { + push @unresolved, [ $_, $lineno ]; } } }
Maybe you are missing show_unresolved() for the last patch here?