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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Aug 6, 2022 at 1:29 PM Fernando Ramos <greenfoo@xxxxxx> 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

I don't know why anyone would want to do that, but the code interprets
that as the user wanting '1b', which is completely ignored.

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux