| Message ID | 20210109224236.50363-1-davvid@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fixup! mergetool: add automerge configuration | expand |
On Sat, Jan 09, 2021 at 02:42:36PM -0800, David Aguilar wrote: > Replace "\r" with a substituted variable that contains "\r". > Replace "\?" with "\{0,1\}". Nice. I was (very slowly) converging on that as the culprit. Thanks for the elegant fix! I'm running the test suite on Windows and OSX (now that they're set up locally) with this included and I'll send a v10 once complete.
David Aguilar <davvid@gmail.com> writes: > "\r\?" in sed is not portable to macOS and possibly others. > "\r" is not valid on Linux with POSIXLY_CORRECT. > > Replace "\r" with a substituted variable that contains "\r". > Replace "\?" with "\{0,1\}". > > Helped-by: brian m. carlson <sandals@crustytoothpaste.net> > Signed-off-by: David Aguilar <davvid@gmail.com> > --- > This is based on top of fc/mergetool-automerge and can be squashed in > (with the addition of my sign-off) if desired. > > Changes since last time: > - printf '\r' instead of printf '\x0d' > - The commit message now mentions POSIXLY_CORRECT. > > git-mergetool.sh | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/git-mergetool.sh b/git-mergetool.sh > index a44afd3822..9029a79431 100755 > --- a/git-mergetool.sh > +++ b/git-mergetool.sh > @@ -243,9 +243,16 @@ auto_merge () { > git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3" > if test -s "$DIFF3" > then > - sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\r\?$/,/^>>>>>>> /d' "$DIFF3" >"$BASE" > - sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$DIFF3" >"$LOCAL" > - sed -e '/^<<<<<<< /,/^=======\r\?$/d' -e '/^>>>>>>> /d' "$DIFF3" >"$REMOTE" > + cr=$(printf '\r') > + sed -e '/^<<<<<<< /,/^||||||| /d' \ > + -e "/^=======$cr\{0,1\}$/,/^>>>>>>> /d" \ > + "$DIFF3" >"$BASE" > + sed -e '/^||||||| /,/^>>>>>>> /d' \ > + -e '/^<<<<<<< /d' \ > + "$DIFF3" >"$LOCAL" > + sed -e "/^<<<<<<< /,/^=======$cr\{0,1\}$/d" \ > + -e '/^>>>>>>> /d' \ > + "$DIFF3" >"$REMOTE" > fi > rm -- "$DIFF3" > } I was hoping that we can avoid repetition that can cause bugs with something like diff --git i/git-mergetool.sh w/git-mergetool.sh index a44afd3822..e07dabbf72 100755 --- i/git-mergetool.sh +++ w/git-mergetool.sh @@ -243,9 +243,13 @@ auto_merge () { git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3" if test -s "$DIFF3" then - sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\r\?$/,/^>>>>>>> /d' "$DIFF3" >"$BASE" - sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$DIFF3" >"$LOCAL" - sed -e '/^<<<<<<< /,/^=======\r\?$/d' -e '/^>>>>>>> /d' "$DIFF3" >"$REMOTE" + C0="^<<<<<<< " + C1="^||||||| " + C2="^=======$(printf '\015')*\$" + C3="^>>>>>>> " + sed -e "/$C0/,/$C1/d" -e "/$C2/,/$C3/d" "$DIFF3" >"$BASE" + sed -e "/$C1/,/$C3/d" -e "/$C0/d" "$DIFF3" >"$LOCAL" + sed -e "/$C0/,/$C2/d" -e "/$C3 /d" "$DIFF3" >"$REMOTE" fi rm -- "$DIFF3" }
Seth House <seth@eseth.com> writes: > On Sat, Jan 09, 2021 at 02:42:36PM -0800, David Aguilar wrote: >> Replace "\r" with a substituted variable that contains "\r". >> Replace "\?" with "\{0,1\}". > > Nice. I was (very slowly) converging on that as the culprit. Thanks for > the elegant fix! I'm running the test suite on Windows and OSX (now that > they're set up locally) with this included and I'll send a v10 once > complete. Note that the topic fails t7800.5 even with the fix-up (and without these fix-up on a platform with sed that do not need the portability fix-up).
Junio C Hamano <gitster@pobox.com> writes: > I was hoping that we can avoid repetition that can cause bugs with > something like > ... Here is a cleaned-up version that would apply cleanly on top of yours. I suspect that it would make it even easier to follow the logic if the two sed "-e" expressions are swapped for the $LOCAL one. It would clarify that we remove $C0 (separator before ours) and $C1..$C3 (ancestor's and theirs, including the separators) to get the local version. The other two are already described in the correct order. Thanks. git-mergetool.sh | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 9029a79431..ed152a4187 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -243,16 +243,14 @@ auto_merge () { git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3" if test -s "$DIFF3" then - cr=$(printf '\r') - sed -e '/^<<<<<<< /,/^||||||| /d' \ - -e "/^=======$cr\{0,1\}$/,/^>>>>>>> /d" \ - "$DIFF3" >"$BASE" - sed -e '/^||||||| /,/^>>>>>>> /d' \ - -e '/^<<<<<<< /d' \ - "$DIFF3" >"$LOCAL" - sed -e "/^<<<<<<< /,/^=======$cr\{0,1\}$/d" \ - -e '/^>>>>>>> /d' \ - "$DIFF3" >"$REMOTE" + C0="^<<<<<<< " + C1="^||||||| " + C2="^=======$(printf '\015')\{0,1\}$" + C3="^>>>>>>> " + + sed -e "/$C0/,/$C1/d" -e "/$C2/,/$C3/d" "$DIFF3" >"$BASE" + sed -e "/$C1/,/$C3/d" -e "/$C0/d" "$DIFF3" >"$LOCAL" + sed -e "/$C0/,/$C2/d" -e "/$C3/d" "$DIFF3" >"$REMOTE" fi rm -- "$DIFF3" }
diff --git a/git-mergetool.sh b/git-mergetool.sh index a44afd3822..9029a79431 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -243,9 +243,16 @@ auto_merge () { git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3" if test -s "$DIFF3" then - sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======\r\?$/,/^>>>>>>> /d' "$DIFF3" >"$BASE" - sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$DIFF3" >"$LOCAL" - sed -e '/^<<<<<<< /,/^=======\r\?$/d' -e '/^>>>>>>> /d' "$DIFF3" >"$REMOTE" + cr=$(printf '\r') + sed -e '/^<<<<<<< /,/^||||||| /d' \ + -e "/^=======$cr\{0,1\}$/,/^>>>>>>> /d" \ + "$DIFF3" >"$BASE" + sed -e '/^||||||| /,/^>>>>>>> /d' \ + -e '/^<<<<<<< /d' \ + "$DIFF3" >"$LOCAL" + sed -e "/^<<<<<<< /,/^=======$cr\{0,1\}$/d" \ + -e '/^>>>>>>> /d' \ + "$DIFF3" >"$REMOTE" fi rm -- "$DIFF3" }
"\r\?" in sed is not portable to macOS and possibly others. "\r" is not valid on Linux with POSIXLY_CORRECT. Replace "\r" with a substituted variable that contains "\r". Replace "\?" with "\{0,1\}". Helped-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: David Aguilar <davvid@gmail.com> --- This is based on top of fc/mergetool-automerge and can be squashed in (with the addition of my sign-off) if desired. Changes since last time: - printf '\r' instead of printf '\x0d' - The commit message now mentions POSIXLY_CORRECT. git-mergetool.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)