Re: [PATCH 0/2] mergetools: vimdiff3: fix regression

From

Felipe Contreras <felipe.contreras@gmail.com>

Date

Aug 6, 2022, 19:17 UTC

Message-ID

<CAMP44s3-RG5k4ZkhAFG_9JtbxcyDhkUmeBh0jCH9+Xwyumyu9w@mail.gmail.com>

In-Reply-To

<Yu6zEiknXKFMJUVn@zacax395.localdomain>

On Sat, Aug 6, 2022 at 1:29 PM Fernando Ramos <greenfoo@u92.eu> wrote:

>
> On 22/08/06 12:53PM, Felipe Contreras wrote:
> > Two observations though.
> >
> > 1. The "silent 4b" is ignored, since bufdo makes the last buffer the
> > current buffer, so if you want a different buffer you have to make the
> > switch *after* bufdo.
> >
>
> Yes, you are right. For the particular case where there are no windows (only
> hidden buffers) it does not have any effect. It's presence there comes from
> the fact that the command generation function works in the most "generic" way
> (ie. producing output that works for all cases: windows, tabs and buffers).
>
> In order not to have another special case in the generation logic I left it
> there, but you are right in that it is not needed (fortunately it also doesn't
> make any harm :)

That's not my point. vimdiff3 is essentially the same as vimdiff with:

    git config --global mergetool.vimdiff.layout MERGED

But the code is written in such a way as to allow:

    git config --global mergetool.vimdiff.layout LOCAL

If we are not going to care about these cases, we can just remove all this code:

--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -254,30 +254,7 @@ gen_cmd_aux () {

        # Step 4:
        #
-       # If we reach this point, it means there are no separators and we just
-       # need to print the command to display the specified buffer
-
-       target=$(substring "$LAYOUT" "$start" "$(( end - start ))" |
sed 's:[ @();|-]::g')
-
-       if test "$target" = "LOCAL"
-       then
-               CMD="$CMD | 1b"
-
-       elif test "$target" = "BASE"
-       then
-               CMD="$CMD | 2b"
-
-       elif test "$target" = "REMOTE"
-       then
-               CMD="$CMD | 3b"
-
-       elif test "$target" = "MERGED"
-       then
-               CMD="$CMD | 4b"
-
-       else
-               CMD="$CMD | ERROR: >$target<"
-       fi
+       # If we reach this point, it means there are no separators.

        echo "$CMD"
        return

> > I don't see the need for all this complexity for this simple mode, but
> > anything that actually works is fine by me.
>
> ...in fact, back in May I just wanted to add a new "vimdiff4" mode and what
> originally was a 5 lines patch became the current 1000+ lines patch monster
> after all the (very welcomed, I'm not complaining!) suggestions :)

I understand the need if you want a complex layout, like
"MERGED+LOCAL,BASE,REMOTE", that's very nice, but if you just want
"MERGED", most of the code does nothing, the extra -c "tabfirst" isn't
needed either.

Either way, adding the silent stuff and "set hidden" make vimdiff3
work, which is all I care about.

Cheers.

-- 
Felipe Contreras