Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> It doesn't make sense to display already-resolved conflicts in the
> different views of all mergetools.
>
> We already have the best version in MERGED, with annotations that can
> be used to extract a pruned version of LOCAL and REMOTE. If we are using
> the diff3 conflict-style, we can even extract BASE.
>
> Let's use these annotations instead of using the original files before
> the conflict resolution.
>
> TODO: There may be a better way to extract these files that doesn't rely
> on the user's conflict-style configuration.
>
> See Seth House's blog post [1] for the idea and the rationale.
>
> [1] https://www.eseth.org/2020/mergetools.html
>
> Cc: Seth House <seth@eseth.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Hmph, I got what Seth showed, but I do not quite see how the ideas
in the post relate to what this patch does.  The patch just avoids
grabbing the contents of each stage out to a file for three stages
using "git checkout-index" and instead does the same by munging the
diff3 output, which ought to have the same information at least for
text files, using "sed", or is there something I am not seeing?

If there is none, then what is the benefit of doing the same thing
without running 3 checkout-index?  Performance?

Also using checkout-index to grab the raw contents without relying
on the textual context marker would mean that a custom mergetool
backend could be used to merge non-text files.  So if this
optimization (I am still assuming this is "doing the same without
running three checkout-index to gain performance" I read out of the
patch text) is worth doing, there needs a knob to allow users to opt
out of it somehow to avoid regressing on the feature front.

If the mergetool were in control to (re)start the merge process that
would result in the conflicted state, we could override the end-user
preference on the conflict style (either 'GNU merge'-style or
'diff3'-style) and conflict-marker-length to be used in showing
textual conflicts, but as I understand "mergetool" is handed an
already conflicted state and asked to resolve it, it would not be
possible without at least looking at the stage #1 to recover the
base for folks who do not use diff3 style.

> ---
>  git-mergetool.sh | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index e3f6d543fb..4759433d46 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -227,18 +227,6 @@ stage_submodule () {
>  	git update-index --add --replace --cacheinfo 160000 "$submodule_sha1" "${work_rel_path%/}" || die
>  }
>  
> -checkout_staged_file () {
> -	tmpfile="$(git checkout-index --temp --stage="$1" "$2" 2>/dev/null)" &&
> -	tmpfile=${tmpfile%%'	'*}
> -
> -	if test $? -eq 0 && test -n "$tmpfile"
> -	then
> -		mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
> -	else
> -		>"$3"
> -	fi
> -}
> -
>  merge_file () {
>  	MERGED="$1"
>  
> @@ -318,9 +306,10 @@ merge_file () {
>  	# where the base's directory no longer exists.
>  	mkdir -p "$(dirname "$MERGED")"
>  
> -	checkout_staged_file 1 "$MERGED" "$BASE"
> -	checkout_staged_file 2 "$MERGED" "$LOCAL"
> -	checkout_staged_file 3 "$MERGED" "$REMOTE"
> +	# TODO: How do we get $MERGED always with diff3?
> +	sed -e '/^<<<<<<< /,/^||||||| /d' -e '/^=======$/,/^>>>>>>> /d' "$MERGED" > "$BASE"
> +	sed -e '/^<<<<<<< /,/^=======$/d' -e '/^>>>>>>> /d' "$MERGED" > "$LOCAL"
> +	sed -e '/^||||||| /,/^>>>>>>> /d' -e '/^<<<<<<< /d' "$MERGED" > "$REMOTE"
>  

Style (lose SP after '>' redirection; I looked at the existing code
and spotted one existing violation but that is not an excuse to make
things even less consistent).

Also, conflict-marker-size=<N> attributes can change the length, so
hardcoding these patterns would not work for everybody.

>  	if test -z "$local_mode" || test -z "$remote_mode"
>  	then

Thanks.