diff mbox series

[1/2] mergetools: vimdiff: fix layout where REMOTE is the target

Message ID 20250325222311.400748-2-greenfoo@u92.eu (mailing list archive)
State Accepted
Commit e2d74193c0e74666498004f5ef7a3473e8993063
Headers show
Series Fix mergetool.vimdiff.layout when "@" is used on REMOTE | expand

Commit Message

Fernando March 25, 2025, 10:23 p.m. UTC
"mergetool.vimdiff.layout" is used to define the vim layout (ie. how
windows, tabs and buffers are physically organized) when resolving
conflicts.

For example, if we set it to this:

    "(LOCAL,BASE,REMOTE)/MERGED"

...vim will open and show this layout:

    ------------------------------------------
    |             |           |              |
    |   LOCAL     |   BASE    |   REMOTE     |
    |             |           |              |
    ------------------------------------------
    |                                        |
    |                MERGED                  |
    |                                        |
    ------------------------------------------

By default, whatever ends up been written to the "MERGED" window will
become the file which conflict we are resolving.

However, it is possible to use the "@" symbol to specify a different
one.  For example, if we use this slightly different version of the
previously used string:

    "(LOCAL,BASE,@REMOTE)/MERGED"

...then the user should proceed to edit the contents of the top right
window (instead of the bottom window) as *that* is what will become the
conflicts free file once vim is closed.

Before this commit, the "@" marker worked for all targets *except* for
"REMOTE". In other words, these worked as expected:

    "(@LOCAL,BASE,REMOTE)/MERGED"
    "(LOCAL,@BASE,REMOTE)/MERGED"
    "(LOCAL,BASE,REMOTE)/@MERGED"

...but this didn't:

    "(LOCAL,BASE,@REMOTE)/MERGED"

This commit fixes that.

Reported-by: kawarimidoll <kawarimidoll+git@gmail.com>
Suggested-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 mergetools/vimdiff | 3 +++
 1 file changed, 3 insertions(+)

Comments

D. Ben Knoble March 29, 2025, 12:23 a.m. UTC | #1
On Tue, Mar 25, 2025 at 6:24 PM Fernando Ramos <greenfoo@u92.eu> wrote:
>
> "mergetool.vimdiff.layout" is used to define the vim layout (ie. how
> windows, tabs and buffers are physically organized) when resolving
> conflicts.
>
> For example, if we set it to this:
>
>     "(LOCAL,BASE,REMOTE)/MERGED"
>
> ...vim will open and show this layout:
>
>     ------------------------------------------
>     |             |           |              |
>     |   LOCAL     |   BASE    |   REMOTE     |
>     |             |           |              |
>     ------------------------------------------
>     |                                        |
>     |                MERGED                  |
>     |                                        |
>     ------------------------------------------
>
> By default, whatever ends up been written to the "MERGED" window will
> become the file which conflict we are resolving.
>
> However, it is possible to use the "@" symbol to specify a different
> one.  For example, if we use this slightly different version of the
> previously used string:
>
>     "(LOCAL,BASE,@REMOTE)/MERGED"
>
> ...then the user should proceed to edit the contents of the top right
> window (instead of the bottom window) as *that* is what will become the
> conflicts free file once vim is closed.
>
> Before this commit, the "@" marker worked for all targets *except* for
> "REMOTE". In other words, these worked as expected:
>
>     "(@LOCAL,BASE,REMOTE)/MERGED"
>     "(LOCAL,@BASE,REMOTE)/MERGED"
>     "(LOCAL,BASE,REMOTE)/@MERGED"
>
> ...but this didn't:
>
>     "(LOCAL,BASE,@REMOTE)/MERGED"
>
> This commit fixes that.
>
> Reported-by: kawarimidoll <kawarimidoll+git@gmail.com>
> Suggested-by: D. Ben Knoble <ben.knoble@gmail.com>
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
>  mergetools/vimdiff | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> index ffc9be86c8..0e3785d230 100644
> --- a/mergetools/vimdiff
> +++ b/mergetools/vimdiff
> @@ -305,6 +305,9 @@ gen_cmd () {
>         elif echo "$LAYOUT" | grep @BASE >/dev/null
>         then
>                 FINAL_TARGET="BASE"
> +       elif echo "$LAYOUT" | grep @REMOTE >/dev/null
> +       then
> +               FINAL_TARGET="REMOTE"
>         else
>                 FINAL_TARGET="MERGED"
>         fi
> --
> 2.49.0
>

This looks pretty obviously correct to me, thanks!
Junio C Hamano March 29, 2025, 8:46 p.m. UTC | #2
"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

>> ...
>>                 FINAL_TARGET="BASE"
>> +       elif echo "$LAYOUT" | grep @REMOTE >/dev/null
>> +       then
>> +               FINAL_TARGET="REMOTE"
>>         else
>>                 FINAL_TARGET="MERGED"
>>         fi
>> --
>> 2.49.0
>>
>
> This looks pretty obviously correct to me, thanks!

Yeah, thanks, all.  Will queue.
diff mbox series

Patch

diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index ffc9be86c8..0e3785d230 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -305,6 +305,9 @@  gen_cmd () {
 	elif echo "$LAYOUT" | grep @BASE >/dev/null
 	then
 		FINAL_TARGET="BASE"
+	elif echo "$LAYOUT" | grep @REMOTE >/dev/null
+	then
+		FINAL_TARGET="REMOTE"
 	else
 		FINAL_TARGET="MERGED"
 	fi