Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable auto-merge for meld to follow the vimdiff beharior #781

Closed
wants to merge 1 commit into from
Closed

Enable auto-merge for meld to follow the vimdiff beharior #781

wants to merge 1 commit into from

Conversation

sunlin7
Copy link
Contributor

@sunlin7 sunlin7 commented May 7, 2020

Hi, the mergetool "meld" does NOT merge the no-conflict changes, while
the mergetool "vimdiff" will merge the no-conflict changes and highlight
the conflict parts. This patch will make the mergetool "meld" similar
to "vimdiff", auto-merge the no-conflict changes, highlight conflict
parts.

cc: Lin Sun lin.sun@zoom.us

@gitgitgadget-git
Copy link

Welcome to GitGitGadget

Hi @sunlin7, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that your Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the FreeNode IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

curl -g --user "<EMailAddress>:<Password>" \
    --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Freenode. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

@gitgitgadget-git
Copy link

There is an issue in commit 5f873da042c7b8b610611af01edfa95f7e181c95:
Commit checks stopped - the message is too short

@gitgitgadget-git
Copy link

There is an issue in commit d38e8f93d8383b6c7ccf37edc8c1389fe1c0bb0c:
Commit not signed off

@dscho
Copy link
Member

dscho commented May 7, 2020

/allow

@gitgitgadget-git
Copy link

User sunlin7 is now allowed to use GitGitGadget.

@sunlin7
Copy link
Contributor Author

sunlin7 commented May 8, 2020

/submit

@gitgitgadget-git
Copy link

@sunlin7
Copy link
Contributor Author

sunlin7 commented May 10, 2020

merged, thanks all of you.

@sunlin7 sunlin7 closed this May 10, 2020
@dscho
Copy link
Member

dscho commented May 10, 2020

I don't see this merged...

@sunlin7 sunlin7 reopened this May 11, 2020
@sunlin7
Copy link
Contributor Author

sunlin7 commented May 11, 2020

awkward... I'll wait for the merge finished, then close this ticket. Thank you @dscho.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jun 8, 2020

Hi @dscho, it seems no event happened for this PR since May 7.
Should I re-submit or do something else to trigger the submit routine?
Thank you.

@dscho
Copy link
Member

dscho commented Jun 8, 2020

If I were you, I would reply on the mailing list with a gentle ping.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jun 8, 2020

The mailing list is updated, look forward for merging.

@gitgitgadget-git
Copy link

On the Git mailing list, Pratyush Yadav wrote (reply to this):

Hi Lin,

I'm not familiar with the code so I'll let someone else comment on that. 
But...

On 08/05/20 01:25AM, sunlin via GitGitGadget wrote:
> From: "lin.sun" <lin.sun@zoom.us>
> 
> The mergetool "meld" does NOT merge the no-conflict changes, while the
> mergetool "vimdiff" will merge the no-conflict parts and highlight the
> conflict parts.
> This patch will make the mergetool "meld" similar to "vimdiff",
> auto-merge the no-conflict parts, highlight conflict parts.
> 
> Signed-off-by: Lin Sun <sunlin7@yahoo.com>

... your name and email in "From:" and "Signed-off-by:" should be the 
same. So either use "lin.sun" <lin.sun@zoom.us> in both places or use 
Lin Sun <sunlin7@yahoo.com> in both.

-- 
Regards,
Pratyush Yadav

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jun 9, 2020

/submit

@gitgitgadget-git
Copy link

@gitgitgadget-git
Copy link

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jun 29, 2020

/submit

@gitgitgadget-git
Copy link

Error: 3b70fd0 was already submitted

@dscho
Copy link
Member

dscho commented Jun 29, 2020

Error: 3b70fd0 was already submitted

@sunlin7 did you receive the email correctly?

@gitgitgadget-git
Copy link

On the Git mailing list, Git Gadget wrote (reply to this):

Re-sending, as this mail seems to have been dropped by vger.

---------- Forwarded message ---------
From: sunlin via GitGitGadget <gitgitgadget@gmail.com>
Date: Mon, Jun 29, 2020 at 9:07 AM
Subject: [PATCH v3] Enable auto-merge for meld to follow the vim-diff beharior
To: <git@vger.kernel.org>
Cc: sunlin <sunlin7@yahoo.com>, lin.sun <lin.sun@zoom.us>


From: "lin.sun" <lin.sun@zoom.us>

The mergetool "meld" does NOT merge the no-conflict changes, while the
mergetool "vimdiff" will merge the no-conflict parts and highlight the
conflict parts.
This patch will make the mergetool "meld" similar to "vimdiff",
auto-merge the no-conflict parts, highlight conflict parts.

Signed-off-by: Lin Sun <lin.sun@zoom.us>
---
    Enable auto-merge for meld to follow the vimdiff beharior

    Hi, the mergetool "meld" does NOT merge the no-conflict changes, while
    the mergetool "vimdiff" will merge the no-conflict changes and highlight
    the conflict parts. This patch will make the mergetool "meld" similar to
    "vimdiff", auto-merge the no-conflict changes, highlight conflict parts.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-781%2Fsunlin7%2Fmaster-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-git-781/sunlin7/master-v3
Pull-Request: https://github.com/git/git/pull/781

Range-diff vs v2:

 1:  6e98a10bfa ! 1:  3b70fd0bfc Enable auto-merge for meld to follow
the vim-diff beharior
     @@ Commit message

       ## mergetools/meld ##
      @@ mergetools/meld: merge_cmd () {
     +  then
     +          check_meld_for_output_version
     +  fi
     ++ if test -z "${meld_has_auto_merge_option:+set}"
     ++ then
     ++         check_meld_for_auto_merge_version
     ++ fi
     ++
     ++ option_auto_merge=
     ++ if test "$meld_has_auto_merge_option" = true
     ++ then
     ++         option_auto_merge="--auto-merge"
     ++ fi

        if test "$meld_has_output_option" = true
        then
      -         "$merge_tool_path" --output="$MERGED" \
     -+         "$merge_tool_path" --auto-merge --output="$MERGED" \
     ++         "$merge_tool_path" $option_auto_merge --output="$MERGED" \
                        "$LOCAL" "$BASE" "$REMOTE"
        else
      -         "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
     -+         "$merge_tool_path" --auto-merge "$LOCAL" "$MERGED" "$REMOTE"
     ++         "$merge_tool_path" $option_auto_merge "$LOCAL"
"$MERGED" "$REMOTE"
        fi
       }

     +@@ mergetools/meld: check_meld_for_output_version () {
     +          meld_has_output_option=false
     +  fi
     + }
     ++
     ++# Check whether we should use 'meld --auto-merge ...'
     ++check_meld_for_auto_merge_version () {
     ++ meld_path="$(git config mergetool.meld.path)"
     ++ meld_path="${meld_path:-meld}"
     ++
     ++ if meld_has_auto_merge_option=$(git config --bool
mergetool.meld.hasAutoMerge)
     ++ then
     ++         : use configured value
     ++ elif "$meld_path" --help 2>&1 |
     ++         grep -e '--auto-merge' -e '\[OPTION\.\.\.\]' >/dev/null
     ++ then
     ++         : old ones mention --auto-merge and new ones just say OPTION...
     ++         meld_has_auto_merge_option=true
     ++ else
     ++         meld_has_auto_merge_option=false
     ++ fi
     ++}


 mergetools/meld | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/mergetools/meld b/mergetools/meld
index 7a08470f88..91b65ff22c 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -7,13 +7,23 @@ merge_cmd () {
        then
                check_meld_for_output_version
        fi
+       if test -z "${meld_has_auto_merge_option:+set}"
+       then
+               check_meld_for_auto_merge_version
+       fi
+
+       option_auto_merge=
+       if test "$meld_has_auto_merge_option" = true
+       then
+               option_auto_merge="--auto-merge"
+       fi

        if test "$meld_has_output_option" = true
        then
-               "$merge_tool_path" --output="$MERGED" \
+               "$merge_tool_path" $option_auto_merge --output="$MERGED" \
                        "$LOCAL" "$BASE" "$REMOTE"
        else
-               "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+               "$merge_tool_path" $option_auto_merge "$LOCAL"
"$MERGED" "$REMOTE"
        fi
 }

@@ -34,3 +44,21 @@ check_meld_for_output_version () {
                meld_has_output_option=false
        fi
 }
+
+# Check whether we should use 'meld --auto-merge ...'
+check_meld_for_auto_merge_version () {
+       meld_path="$(git config mergetool.meld.path)"
+       meld_path="${meld_path:-meld}"
+
+       if meld_has_auto_merge_option=$(git config --bool
mergetool.meld.hasAutoMerge)
+       then
+               : use configured value
+       elif "$meld_path" --help 2>&1 |
+               grep -e '--auto-merge' -e '\[OPTION\.\.\.\]' >/dev/null
+       then
+               : old ones mention --auto-merge and new ones just say OPTION...
+               meld_has_auto_merge_option=true
+       else
+               meld_has_auto_merge_option=false
+       fi
+}

base-commit: 07d8ea56f2ecb64b75b92264770c0a664231ce17
--
gitgitgadget

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

"sunlin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: "lin.sun" <lin.sun@zoom.us>

The way this "name <addr>" is spelled must match the name used on
the Signed-off-by: line below.

> The mergetool "meld" does NOT merge the no-conflict changes, while the
> mergetool "vimdiff" will merge the no-conflict parts and highlight the
> conflict parts.

OK.

Have a blank line between paragraphs here.

> This patch will make the mergetool "meld" similar to "vimdiff",
> auto-merge the no-conflict parts, highlight conflict parts.

Give an order to the codebase to be like so, e.g.:

    Make the mergetool used with "meld" backend behave similarly to
    how "vimdiff" beheaves by telling it to auto-merge parts without
    conflicts and highlight the parts with conflicts.

or somethin glike that.

> Signed-off-by: Lin Sun <lin.sun@zoom.us>
> ---
>     Enable auto-merge for meld to follow the vimdiff beharior
>     
>     Hi, the mergetool "meld" does NOT merge the no-conflict changes, while
>     the mergetool "vimdiff" will merge the no-conflict changes and highlight
>     the conflict parts. This patch will make the mergetool "meld" similar to
>     "vimdiff", auto-merge the no-conflict changes, highlight conflict parts.

That repeats the log message and does not add much useful info.

>  mergetools/meld | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/mergetools/meld b/mergetools/meld
> index 7a08470f88..91b65ff22c 100644
> --- a/mergetools/meld
> +++ b/mergetools/meld
> @@ -7,13 +7,23 @@ merge_cmd () {
>  	then
>  		check_meld_for_output_version
>  	fi
> +	if test -z "${meld_has_auto_merge_option:+set}"
> +	then
> +		check_meld_for_auto_merge_version
> +	fi

The detection part looks clumsy and inefficient.  More about it later.

> +	option_auto_merge=
> +	if test "$meld_has_auto_merge_option" = true
> +	then
> +		option_auto_merge="--auto-merge"
> +	fi
>  
>  	if test "$meld_has_output_option" = true
>  	then
> -		"$merge_tool_path" --output="$MERGED" \
> +		"$merge_tool_path" $option_auto_merge --output="$MERGED" \
>  			"$LOCAL" "$BASE" "$REMOTE"
>  	else
> -		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
> +		"$merge_tool_path" $option_auto_merge "$LOCAL" "$MERGED" "$REMOTE"
>  	fi
>  }

The part that chooses whether to pass --auto-merge or not and
adjusts the command line options does look sensible.

I wonder if the same "hasAutoMerge" option can be used by those who
do *not* want the new --auto-merge behaviour to opt out of this
change.  Is there a reason why "meld" offers the --auto-merge as an
optional behaviour (which tells, at least to me, that the default
behaviour is not to auto-merge and makes me assume that the default
must be chosen by some sound reasoning, hence some users would prefer
not to use this new behaviour with good reasons)?

I guess what I am trying to get at is, if --auto-merge is an optional
and non-default behaviour for "meld" users, perhaps it is not a good
idea to change the behaviour on them only because the version of meld
they run happens to support the --auto-merge as an optional behaviour.

IOW, wouldn't it make more sense, and certainly make it safer
without surprises to existing users, if we made the logic to

    * If mergetool.meld.useAutoMerge is not set, do not pass
      --auto-merge whether "meld" supports the option or not.

    * If mergetool.meld.useAutoMerge is 'true', always pass it
      without checking.

    * If mergetool.meld.useAutoMerge is 'when-able' (or come up with
      a better name if you want, perhaps 'auto'), check if "meld"
      accepts "--auto-merge" and decide whether to pass it or not.

perhaps?

> +# Check whether we should use 'meld --auto-merge ...'
> +check_meld_for_auto_merge_version () {
> +	meld_path="$(git config mergetool.meld.path)"
> +	meld_path="${meld_path:-meld}"
> +
> +	if meld_has_auto_merge_option=$(git config --bool mergetool.meld.hasAutoMerge)
> +	then
> +		: use configured value
> +	elif "$meld_path" --help 2>&1 |
> +		grep -e '--auto-merge' -e '\[OPTION\.\.\.\]' >/dev/null
> +	then
> +		: old ones mention --auto-merge and new ones just say OPTION...
> +		meld_has_auto_merge_option=true
> +	else
> +		meld_has_auto_merge_option=false
> +	fi
> +}

When not configured, we end up running "meld --help" twice for two
options, which is not great, don't you think?  I actually think the
part that runs "meld --help" and parses its output should be split
out of the helper function "check_meld_for_output_version" and
called "check_meld_supported_options" or something, so that the
logic to see if the --output and --auto-merge options should be
passed can be made with at most one invocation of "meld --help".
Which may involve *NOT* having two separate helper functions
check_meld_for_*_version for the tested features.

Oh, also, check_meld_for_*_version is nonsensical as a name for
these helper functions (it is not the fault of this patch---it is
mimicking the existing practice, but that does not make the function
name not nonsensical).  The helpers do not actually want to check a
"version", they only want to see if a feature is supported.  So
having "feature" in the name would be good, but "version" is not.



@gitgitgadget-git
Copy link

On the Git mailing list, David Aguilar wrote (reply to this):

On Mon, Jun 29, 2020 at 05:06:13PM -0700, Junio C Hamano wrote:
> "sunlin via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >  mergetools/meld | 32 ++++++++++++++++++++++++++++++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/mergetools/meld b/mergetools/meld
> > index 7a08470f88..91b65ff22c 100644
> > --- a/mergetools/meld
> > +++ b/mergetools/meld
> > @@ -7,13 +7,23 @@ merge_cmd () {
> >  	then
> >  		check_meld_for_output_version
> >  	fi
> > +	if test -z "${meld_has_auto_merge_option:+set}"
> > +	then
> > +		check_meld_for_auto_merge_version
> > +	fi
> 
> The detection part looks clumsy and inefficient.  More about it later.


Sorry for not noticing your reply here earlier.  I agree with everything
you wrote here, and rescind my earlier sign-off.  Combining as you
suggested below is best IMO as well.

> 
> > +	option_auto_merge=
> > +	if test "$meld_has_auto_merge_option" = true
> > +	then
> > +		option_auto_merge="--auto-merge"
> > +	fi
> >  
> >  	if test "$meld_has_output_option" = true
> >  	then
> > -		"$merge_tool_path" --output="$MERGED" \
> > +		"$merge_tool_path" $option_auto_merge --output="$MERGED" \
> >  			"$LOCAL" "$BASE" "$REMOTE"
> >  	else
> > -		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
> > +		"$merge_tool_path" $option_auto_merge "$LOCAL" "$MERGED" "$REMOTE"
> >  	fi
> >  }
> 
> The part that chooses whether to pass --auto-merge or not and
> adjusts the command line options does look sensible.
> 
> I wonder if the same "hasAutoMerge" option can be used by those who
> do *not* want the new --auto-merge behaviour to opt out of this
> change.  Is there a reason why "meld" offers the --auto-merge as an
> optional behaviour (which tells, at least to me, that the default
> behaviour is not to auto-merge and makes me assume that the default
> must be chosen by some sound reasoning, hence some users would prefer
> not to use this new behaviour with good reasons)?
> 
> I guess what I am trying to get at is, if --auto-merge is an optional
> and non-default behaviour for "meld" users, perhaps it is not a good
> idea to change the behaviour on them only because the version of meld
> they run happens to support the --auto-merge as an optional behaviour.
> 
> IOW, wouldn't it make more sense, and certainly make it safer
> without surprises to existing users, if we made the logic to
> 
>     * If mergetool.meld.useAutoMerge is not set, do not pass
>       --auto-merge whether "meld" supports the option or not.
> 
>     * If mergetool.meld.useAutoMerge is 'true', always pass it
>       without checking.
> 
>     * If mergetool.meld.useAutoMerge is 'when-able' (or come up with
>       a better name if you want, perhaps 'auto'), check if "meld"
>       accepts "--auto-merge" and decide whether to pass it or not.
> 
> perhaps?


I like the idea of having it be auto/true/false, and perhaps "auto"
would be a sensible default if more users benefit from it than not.

Sunlin, do you have an opinion on what the default should be?



> > +# Check whether we should use 'meld --auto-merge ...'
> > +check_meld_for_auto_merge_version () {
> > +	meld_path="$(git config mergetool.meld.path)"

Small sug -- this command substitution doesn't need the enclosing
quotes.

	meld_path=$(git config mergetool.meld.path || echo meld)

should be sufficient.  Are we okay with `|| echo meld`?
If so, it would let us drop this line below.

> > +	meld_path="${meld_path:-meld}"


> > +
> > +	if meld_has_auto_merge_option=$(git config --bool mergetool.meld.hasAutoMerge)
> > +	then
> > +		: use configured value
> > +	elif "$meld_path" --help 2>&1 |
> > +		grep -e '--auto-merge' -e '\[OPTION\.\.\.\]' >/dev/null
> > +	then
> > +		: old ones mention --auto-merge and new ones just say OPTION...
> > +		meld_has_auto_merge_option=true
> > +	else
> > +		meld_has_auto_merge_option=false
> > +	fi
> > +}
> 
> When not configured, we end up running "meld --help" twice for two
> options, which is not great, don't you think?  I actually think the
> part that runs "meld --help" and parses its output should be split
> out of the helper function "check_meld_for_output_version" and
> called "check_meld_supported_options" or something, so that the
> logic to see if the --output and --auto-merge options should be
> passed can be made with at most one invocation of "meld --help".
> Which may involve *NOT* having two separate helper functions
> check_meld_for_*_version for the tested features.


I'm 100% on board with this suggestion.

> 
> Oh, also, check_meld_for_*_version is nonsensical as a name for
> these helper functions (it is not the fault of this patch---it is
> mimicking the existing practice, but that does not make the function
> name not nonsensical).  The helpers do not actually want to check a
> "version", they only want to see if a feature is supported.  So
> having "feature" in the name would be good, but "version" is not.


Ditto.
-- 
David

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jun 30, 2020

/submit

@gitgitgadget-git
Copy link

@gitgitgadget-git
Copy link

On the Git mailing list, wrote (reply to this):

This is a multipart message in MIME format.

------=_NextPart_000_F61B_01D64F14.2C5A8B80
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi David, Junio,

Appreciate for your comments, I rewrite the "mergetool/meld" to follow =
your comments and suggestions.
It will respect the git config first, then detect the options if no =
configuration for them, and also reduce the subprocess calling.
Both the modified-file and patch-file are appended.
Please review again. Thanks

Regards
Lin Sun

------=_NextPart_000_F61B_01D64F14.2C5A8B80
Content-Type: application/octet-stream;
	name="0001-Enable-auto-merge-for-meld-to-follow-the-vim-diff-be.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="0001-Enable-auto-merge-for-meld-to-follow-the-vim-diff-be.patch"

From 93ae3ec011e7b55cc9971f91345cb4aab5189cb4 Mon Sep 17 00:00:00 2001=0A=
From: "lin.sun" <lin.sun@zoom.us>=0A=
Date: Thu, 7 May 2020 07:31:14 +0800=0A=
Subject: [PATCH] Enable auto-merge for meld to follow the vim-diff =
beharior=0A=
=0A=
Make the mergetool used with "meld" backend behave similarly to=0A=
how "vimdiff" beheaves by telling it to auto-merge parts without=0A=
conflicts and highlight the parts with conflicts.=0A=
=0A=
Signed-off-by: lin.sun <lin.sun@zoom.us>=0A=
---=0A=
 mergetools/meld | 73 =
++++++++++++++++++++++++++++++++++++++++++++-------------=0A=
 1 file changed, 57 insertions(+), 16 deletions(-)=0A=
=0A=
diff --git a/mergetools/meld b/mergetools/meld=0A=
index 7a08470..1b92771 100644=0A=
--- a/mergetools/meld=0A=
+++ b/mergetools/meld=0A=
@@ -3,34 +3,75 @@ diff_cmd () {=0A=
 }=0A=
 =0A=
 merge_cmd () {=0A=
-	if test -z "${meld_has_output_option:+set}"=0A=
+	check_meld_for_features=0A=
+=0A=
+	option_auto_merge=3D=0A=
+	if test "$meld_has_auto_merge_option" =3D true=0A=
 	then=0A=
-		check_meld_for_output_version=0A=
+		option_auto_merge=3D"--auto-merge"=0A=
 	fi=0A=
 =0A=
 	if test "$meld_has_output_option" =3D true=0A=
 	then=0A=
-		"$merge_tool_path" --output=3D"$MERGED" \=0A=
+		"$merge_tool_path" $option_auto_merge --output=3D"$MERGED" \=0A=
 			"$LOCAL" "$BASE" "$REMOTE"=0A=
 	else=0A=
-		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"=0A=
+		"$merge_tool_path" $option_auto_merge "$LOCAL" "$MERGED" "$REMOTE"=0A=
 	fi=0A=
 }=0A=
 =0A=
-# Check whether we should use 'meld --output <file>'=0A=
-check_meld_for_output_version () {=0A=
-	meld_path=3D"$(git config mergetool.meld.path)"=0A=
-	meld_path=3D"${meld_path:-meld}"=0A=
+# Get meld help message=0A=
+get_meld_help_msg () {=0A=
+	meld_path=3D"$(git config mergetool.meld.path || echo meld)"=0A=
+  $meld_path --help 2>&1=0A=
+}=0A=
 =0A=
-	if meld_has_output_option=3D$(git config --bool =
mergetool.meld.hasOutput)=0A=
+# Check the features and set flags=0A=
+check_meld_for_features () {=0A=
+	# Check whether we should use 'meld --output <file>'=0A=
+	if test -z "${meld_has_output_option:+set}"=0A=
 	then=0A=
-		: use configured value=0A=
-	elif "$meld_path" --help 2>&1 |=0A=
-		grep -e '--output=3D' -e '\[OPTION\.\.\.\]' >/dev/null=0A=
+		meld_has_output_option=3D$(git config --bool mergetool.meld.hasOutput)=0A=
+		if test "$meld_has_output_option" =3D true -o \=0A=
+						"$meld_has_output_option" =3D false=0A=
+		then=0A=
+			: use configured value=0A=
+		else												# treat meld_has_output_option as "auto"=0A=
+			if test -z "$meld_help_msg"=0A=
+			then=0A=
+				meld_help_msg=3D"$(get_meld_help_msg)"=0A=
+			fi=0A=
+=0A=
+			if echo "$meld_help_msg" |=0A=
+							grep -e '--output=3D' -e '\[OPTION\.\.\.\]' >/dev/null=0A=
+			then=0A=
+				: old ones mention --output and new ones just say OPTION...=0A=
+				meld_has_output_option=3Dtrue=0A=
+			else=0A=
+				meld_has_output_option=3Dfalse=0A=
+			fi=0A=
+		fi=0A=
+	fi=0A=
+	# Check whether we should use 'meld --auto-merge ...'=0A=
+	if test -z "${meld_has_auto_merge_option:+set}"=0A=
 	then=0A=
-		: old ones mention --output and new ones just say OPTION...=0A=
-		meld_has_output_option=3Dtrue=0A=
-	else=0A=
-		meld_has_output_option=3Dfalse=0A=
+		meld_has_auto_merge_option=3D$(git config --bool =
mergetool.meld.hasAutoMerge)=0A=
+		if test "$meld_has_auto_merge_option" =3D true -o \=0A=
+						"$meld_has_auto_merge_option" =3D false=0A=
+		then=0A=
+			: use configured value=0A=
+		else												# treat meld_has_auto_merge_option as "auto"=0A=
+			if test -z "$meld_help_msg"=0A=
+			then=0A=
+					meld_help_msg=3D"$(get_meld_help_msg)"=0A=
+			fi=0A=
+=0A=
+			if echo "$meld_help_msg" | grep -e '--auto-merge' >/dev/null=0A=
+			then=0A=
+					meld_has_auto_merge_option=3Dtrue=0A=
+			else=0A=
+				meld_has_auto_merge_option=3Dfalse=0A=
+			fi=0A=
+		fi=0A=
 	fi=0A=
 }=0A=
-- =0A=
2.2.0=0A=
=0A=

------=_NextPart_000_F61B_01D64F14.2C5A8B80
Content-Type: application/octet-stream;
	name="meld"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="meld"

diff_cmd () {=0A=
	"$merge_tool_path" "$LOCAL" "$REMOTE"=0A=
}=0A=
=0A=
merge_cmd () {=0A=
	check_meld_for_features=0A=
=0A=
	option_auto_merge=3D=0A=
	if test "$meld_has_auto_merge_option" =3D true=0A=
	then=0A=
		option_auto_merge=3D"--auto-merge"=0A=
	fi=0A=
=0A=
	if test "$meld_has_output_option" =3D true=0A=
	then=0A=
		"$merge_tool_path" $option_auto_merge --output=3D"$MERGED" \=0A=
			"$LOCAL" "$BASE" "$REMOTE"=0A=
	else=0A=
		"$merge_tool_path" $option_auto_merge "$LOCAL" "$MERGED" "$REMOTE"=0A=
	fi=0A=
}=0A=
=0A=
# Get meld help message=0A=
get_meld_help_msg () {=0A=
	meld_path=3D"$(git config mergetool.meld.path || echo meld)"=0A=
  $meld_path --help 2>&1=0A=
}=0A=
=0A=
# Check the features and set flags=0A=
check_meld_for_features () {=0A=
	# Check whether we should use 'meld --output <file>'=0A=
	if test -z "${meld_has_output_option:+set}"=0A=
	then=0A=
		meld_has_output_option=3D$(git config --bool mergetool.meld.hasOutput)=0A=
		if test "$meld_has_output_option" =3D true -o \=0A=
						"$meld_has_output_option" =3D false=0A=
		then=0A=
			: use configured value=0A=
		else												# treat meld_has_output_option as "auto"=0A=
			if test -z "$meld_help_msg"=0A=
			then=0A=
				meld_help_msg=3D"$(get_meld_help_msg)"=0A=
			fi=0A=
=0A=
			if echo "$meld_help_msg" |=0A=
							grep -e '--output=3D' -e '\[OPTION\.\.\.\]' >/dev/null=0A=
			then=0A=
				: old ones mention --output and new ones just say OPTION...=0A=
				meld_has_output_option=3Dtrue=0A=
			else=0A=
				meld_has_output_option=3Dfalse=0A=
			fi=0A=
		fi=0A=
	fi=0A=
	# Check whether we should use 'meld --auto-merge ...'=0A=
	if test -z "${meld_has_auto_merge_option:+set}"=0A=
	then=0A=
		meld_has_auto_merge_option=3D$(git config --bool =
mergetool.meld.hasAutoMerge)=0A=
		if test "$meld_has_auto_merge_option" =3D true -o \=0A=
						"$meld_has_auto_merge_option" =3D false=0A=
		then=0A=
			: use configured value=0A=
		else												# treat meld_has_auto_merge_option as "auto"=0A=
			if test -z "$meld_help_msg"=0A=
			then=0A=
					meld_help_msg=3D"$(get_meld_help_msg)"=0A=
			fi=0A=
=0A=
			if echo "$meld_help_msg" | grep -e '--auto-merge' >/dev/null=0A=
			then=0A=
					meld_has_auto_merge_option=3Dtrue=0A=
			else=0A=
				meld_has_auto_merge_option=3Dfalse=0A=
			fi=0A=
		fi=0A=
	fi=0A=
}=0A=

------=_NextPart_000_F61B_01D64F14.2C5A8B80--

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jun 30, 2020

@dscho it's my failed that I open two pages and I try submit on both pages.

@gitgitgadget-git
Copy link

On the Git mailing list, wrote (reply to this):

The last change also available in https://github.com/git/git/pull/781/files.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via f152f19.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via 30b6e6d.

Make the mergetool used with "meld" backend behave similarly to "vimdiff" by
telling it to auto-merge non-conflicting parts and highlight the conflicting
parts when `mergetool.meld.useAutoMerge` is configured with `true`, or `auto`
for detecting the `--auto-merge` option automatically.

Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Helped-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Lin Sun <lin.sun@zoom.us>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@gitgitgadget-git
Copy link

This patch series was integrated into seen via 468db80.

@gitgitgadget-git
Copy link

On the Git mailing list, Lin Sun wrote (reply to this):

--0000000000005987f505aed76a61
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Junio,

Sorry again, I have to re-send this mail in plain text mode for the
mail system rejecting it with " <git@vger.kernel.org> Content-Policy
reject msg: The message contains HTML subpart, therefore we
    consider it SPAM or Outlook Virus."
--------------------------------------------------------
I tried to send an update to you in the morning but now the mail
missed from my drafts and =E2=80=9Csend box=E2=80=9D, I=E2=80=99m not sure =
if the mail already
sent or not.
So I sent this mail with
=E2=80=9C0001-Support-auto-merge-for-meld-to-follow-the-vim-diff-b.patch=E2=
=80=9D for
assurement.
If you already received this patch before, please ignore current mail.
--------------------------------------------------------
After applying the changes with your SQUASH??? Commit, I test the
cases with useAutoMerge flag None/true/false/auto, it works like a
charm.
So I sent out the last patch (no changes since your SQUASH commit),
please review it. Thank you.

Best Regards
Lin Sun

--0000000000005987f505aed76a61
Content-Type: application/octet-stream; 
	name="0001-Support-auto-merge-for-meld-to-follow-the-vim-diff-b.patch"
Content-Disposition: attachment; 
	filename="0001-Support-auto-merge-for-meld-to-follow-the-vim-diff-b.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_keupigj50>
X-Attachment-Id: f_keupigj50

RnJvbSAxYjkzMWU2MTBkZGZjMDJkYzJlODgyMzRiMTA2ZTNkM2ZmODQxYjAxIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBMaW4gU3VuIDxsaW4uc3VuQHpvb20udXM+CkRhdGU6IFRodSwg
NyBNYXkgMjAyMCAwNzozMToxNCArMDgwMApTdWJqZWN0OiBbUEFUQ0hdIFN1cHBvcnQgYXV0by1t
ZXJnZSBmb3IgbWVsZCB0byBmb2xsb3cgdGhlIHZpbS1kaWZmIGJlaGF2aW9yCk1JTUUtVmVyc2lv
bjogMS4wCkNvbnRlbnQtVHlwZTogdGV4dC9wbGFpbjsgY2hhcnNldD1VVEYtOApDb250ZW50LVRy
YW5zZmVyLUVuY29kaW5nOiA4Yml0CgpNYWtlIHRoZSBtZXJnZXRvb2wgdXNlZCB3aXRoICJtZWxk
IiBiYWNrZW5kIGJlaGF2ZSBzaW1pbGFybHkgdG8gInZpbWRpZmYiIGJ5CnRlbGxpbmcgaXQgdG8g
YXV0by1tZXJnZSBub24tY29uZmxpY3RpbmcgcGFydHMgYW5kIGhpZ2hsaWdodCB0aGUgY29uZmxp
Y3RpbmcKcGFydHMgd2hlbiBgbWVyZ2V0b29sLm1lbGQudXNlQXV0b01lcmdlYCBpcyBjb25maWd1
cmVkIHdpdGggYHRydWVgLCBvciBgYXV0b2AKZm9yIGRldGVjdGluZyB0aGUgYC0tYXV0by1tZXJn
ZWAgb3B0aW9uIGF1dG9tYXRpY2FsbHkuCgpIZWxwZWQtYnk6IMSQb8OgbiBUcuG6p24gQ8O0bmcg
RGFuaCA8Y29uZ2RhbmhxeEBnbWFpbC5jb20+CkhlbHBlZC1ieTogRGF2aWQgQWd1aWxhciA8ZGF2
dmlkQGdtYWlsLmNvbT4KU2lnbmVkLW9mZi1ieTogTGluIFN1biA8bGluLnN1bkB6b29tLnVzPgpT
aWduZWQtb2ZmLWJ5OiBKdW5pbyBDIEhhbWFubyA8Z2l0c3RlckBwb2JveC5jb20+Ci0tLQogRG9j
dW1lbnRhdGlvbi9jb25maWcvbWVyZ2V0b29sLnR4dCB8IDEwICsrKysrCiBidWlsdGluL2NvbmZp
Zy5jICAgICAgICAgICAgICAgICAgIHwgMTcgKysrKysrKysKIG1lcmdldG9vbHMvbWVsZCAgICAg
ICAgICAgICAgICAgICAgfCA4NSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrLS0tLS0t
LQogMyBmaWxlcyBjaGFuZ2VkLCA5NiBpbnNlcnRpb25zKCspLCAxNiBkZWxldGlvbnMoLSkKCmRp
ZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2NvbmZpZy9tZXJnZXRvb2wudHh0IGIvRG9jdW1lbnRh
dGlvbi9jb25maWcvbWVyZ2V0b29sLnR4dAppbmRleCAwOWVkMzFkLi4xNmEyNzQ0IDEwMDY0NAot
LS0gYS9Eb2N1bWVudGF0aW9uL2NvbmZpZy9tZXJnZXRvb2wudHh0CisrKyBiL0RvY3VtZW50YXRp
b24vY29uZmlnL21lcmdldG9vbC50eHQKQEAgLTMwLDYgKzMwLDE2IEBAIG1lcmdldG9vbC5tZWxk
Lmhhc091dHB1dDo6CiAJdG8gYHRydWVgIHRlbGxzIEdpdCB0byB1bmNvbmRpdGlvbmFsbHkgdXNl
IHRoZSBgLS1vdXRwdXRgIG9wdGlvbiwKIAlhbmQgYGZhbHNlYCBhdm9pZHMgdXNpbmcgYC0tb3V0
cHV0YC4KIAorbWVyZ2V0b29sLm1lbGQudXNlQXV0b01lcmdlOjoKKwlXaGVuIHRoZSBgLS1hdXRv
LW1lcmdlYCBpcyBnaXZlbiwgbWVsZCB3aWxsIG1lcmdlIGFsbCBub24tY29uZmxpY3RpbmcKKwlw
YXJ0cyBhdXRvbWF0aWNhbGx5LCBoaWdobGlnaHQgdGhlIGNvbmZsaWN0aW5nIHBhcnRzIGFuZCB3
YWl0IGZvcgorCXVzZXIgZGVjaXNpb24uICBTZXR0aW5nIGBtZXJnZXRvb2wubWVsZC51c2VBdXRv
TWVyZ2VgIHRvIGB0cnVlYCB0ZWxscworCUdpdCB0byB1bmNvbmRpdGlvbmFsbHkgdXNlIHRoZSBg
LS1hdXRvLW1lcmdlYCBvcHRpb24gd2l0aCBgbWVsZGAuCisJU2V0dGluZyB0aGlzIHZhbHVlIHRv
IGBhdXRvYCBtYWtlcyBnaXQgZGV0ZWN0IHdoZXRoZXIgYC0tYXV0by1tZXJnZWAKKwlpcyBzdXBw
b3J0ZWQgYW5kIHdpbGwgb25seSB1c2UgYC0tYXV0by1tZXJnZWAgd2hlbiBhdmFpbGFibGUuICBB
CisJdmFsdWUgb2YgYGZhbHNlYCBhdm9pZHMgdXNpbmcgYC0tYXV0by1tZXJnZWAgYWx0b2dldGhl
ciwgYW5kIGlzIHRoZQorCWRlZmF1bHQgdmFsdWUuCisKIG1lcmdldG9vbC5rZWVwQmFja3VwOjoK
IAlBZnRlciBwZXJmb3JtaW5nIGEgbWVyZ2UsIHRoZSBvcmlnaW5hbCBmaWxlIHdpdGggY29uZmxp
Y3QgbWFya2VycwogCWNhbiBiZSBzYXZlZCBhcyBhIGZpbGUgd2l0aCBhIGAub3JpZ2AgZXh0ZW5z
aW9uLiAgSWYgdGhpcyB2YXJpYWJsZQpkaWZmIC0tZ2l0IGEvYnVpbHRpbi9jb25maWcuYyBiL2J1
aWx0aW4vY29uZmlnLmMKaW5kZXggNWUzOWY2MS4uYWUwNzdjOSAxMDA2NDQKLS0tIGEvYnVpbHRp
bi9jb25maWcuYworKysgYi9idWlsdGluL2NvbmZpZy5jCkBAIC02NSw2ICs2NSw3IEBAIHN0YXRp
YyBpbnQgc2hvd19zY29wZTsKICNkZWZpbmUgVFlQRV9QQVRICQk0CiAjZGVmaW5lIFRZUEVfRVhQ
SVJZX0RBVEUJNQogI2RlZmluZSBUWVBFX0NPTE9SCQk2CisjZGVmaW5lIFRZUEVfQk9PTF9PUl9T
VFIJNwogCiAjZGVmaW5lIE9QVF9DQUxMQkFDS19WQUxVRShzLCBsLCB2LCBoLCBpKSBcCiAJeyBP
UFRJT05fQ0FMTEJBQ0ssIChzKSwgKGwpLCAodiksIE5VTEwsIChoKSwgUEFSU0VfT1BUX05PQVJH
IHwgXApAQCAtOTQsNiArOTUsOCBAQCBzdGF0aWMgaW50IG9wdGlvbl9wYXJzZV90eXBlKGNvbnN0
IHN0cnVjdCBvcHRpb24gKm9wdCwgY29uc3QgY2hhciAqYXJnLAogCQkJbmV3X3R5cGUgPSBUWVBF
X0lOVDsKIAkJZWxzZSBpZiAoIXN0cmNtcChhcmcsICJib29sLW9yLWludCIpKQogCQkJbmV3X3R5
cGUgPSBUWVBFX0JPT0xfT1JfSU5UOworCQllbHNlIGlmICghc3RyY21wKGFyZywgImJvb2wtb3It
c3RyIikpCisJCQluZXdfdHlwZSA9IFRZUEVfQk9PTF9PUl9TVFI7CiAJCWVsc2UgaWYgKCFzdHJj
bXAoYXJnLCAicGF0aCIpKQogCQkJbmV3X3R5cGUgPSBUWVBFX1BBVEg7CiAJCWVsc2UgaWYgKCFz
dHJjbXAoYXJnLCAiZXhwaXJ5LWRhdGUiKSkKQEAgLTE0OSw2ICsxNTIsNyBAQCBzdGF0aWMgc3Ry
dWN0IG9wdGlvbiBidWlsdGluX2NvbmZpZ19vcHRpb25zW10gPSB7CiAJT1BUX0NBTExCQUNLX1ZB
TFVFKDAsICJib29sIiwgJnR5cGUsIE5fKCJ2YWx1ZSBpcyBcInRydWVcIiBvciBcImZhbHNlXCIi
KSwgVFlQRV9CT09MKSwKIAlPUFRfQ0FMTEJBQ0tfVkFMVUUoMCwgImludCIsICZ0eXBlLCBOXygi
dmFsdWUgaXMgZGVjaW1hbCBudW1iZXIiKSwgVFlQRV9JTlQpLAogCU9QVF9DQUxMQkFDS19WQUxV
RSgwLCAiYm9vbC1vci1pbnQiLCAmdHlwZSwgTl8oInZhbHVlIGlzIC0tYm9vbCBvciAtLWludCIp
LCBUWVBFX0JPT0xfT1JfSU5UKSwKKwlPUFRfQ0FMTEJBQ0tfVkFMVUUoMCwgImJvb2wtb3Itc3Ry
IiwgJnR5cGUsIE5fKCJ2YWx1ZSBpcyAtLWJvb2wgb3Igc3RyaW5nIiksIFRZUEVfQk9PTF9PUl9T
VFIpLAogCU9QVF9DQUxMQkFDS19WQUxVRSgwLCAicGF0aCIsICZ0eXBlLCBOXygidmFsdWUgaXMg
YSBwYXRoIChmaWxlIG9yIGRpcmVjdG9yeSBuYW1lKSIpLCBUWVBFX1BBVEgpLAogCU9QVF9DQUxM
QkFDS19WQUxVRSgwLCAiZXhwaXJ5LWRhdGUiLCAmdHlwZSwgTl8oInZhbHVlIGlzIGFuIGV4cGly
eSBkYXRlIiksIFRZUEVfRVhQSVJZX0RBVEUpLAogCU9QVF9HUk9VUChOXygiT3RoZXIiKSksCkBA
IC0yNTAsNiArMjU0LDEyIEBAIHN0YXRpYyBpbnQgZm9ybWF0X2NvbmZpZyhzdHJ1Y3Qgc3RyYnVm
ICpidWYsIGNvbnN0IGNoYXIgKmtleV8sIGNvbnN0IGNoYXIgKnZhbHVlCiAJCQkJc3RyYnVmX2Fk
ZHN0cihidWYsIHYgPyAidHJ1ZSIgOiAiZmFsc2UiKTsKIAkJCWVsc2UKIAkJCQlzdHJidWZfYWRk
ZihidWYsICIlZCIsIHYpOworCQl9IGVsc2UgaWYgKHR5cGUgPT0gVFlQRV9CT09MX09SX1NUUikg
eworCQkJaW50IHYgPSBnaXRfcGFyc2VfbWF5YmVfYm9vbCh2YWx1ZV8pOworCQkJaWYgKHYgPCAw
KQorCQkJCXN0cmJ1Zl9hZGRzdHIoYnVmLCB2YWx1ZV8pOworCQkJZWxzZQorCQkJCXN0cmJ1Zl9h
ZGRzdHIoYnVmLCB2ID8gInRydWUiIDogImZhbHNlIik7CiAJCX0gZWxzZSBpZiAodHlwZSA9PSBU
WVBFX1BBVEgpIHsKIAkJCWNvbnN0IGNoYXIgKnY7CiAJCQlpZiAoZ2l0X2NvbmZpZ19wYXRobmFt
ZSgmdiwga2V5XywgdmFsdWVfKSA8IDApCkBAIC00MTEsNiArNDIxLDEzIEBAIHN0YXRpYyBjaGFy
ICpub3JtYWxpemVfdmFsdWUoY29uc3QgY2hhciAqa2V5LCBjb25zdCBjaGFyICp2YWx1ZSkKIAkJ
ZWxzZQogCQkJcmV0dXJuIHhzdHJkdXAodiA/ICJ0cnVlIiA6ICJmYWxzZSIpOwogCX0KKwlpZiAo
dHlwZSA9PSBUWVBFX0JPT0xfT1JfU1RSKSB7CisJCWludCB2ID0gZ2l0X3BhcnNlX21heWJlX2Jv
b2wodmFsdWUpOworCQlpZiAodiA8IDApCisJCQlyZXR1cm4geHN0cmR1cCh2YWx1ZSk7CisJCWVs
c2UKKwkJCXJldHVybiB4c3RyZHVwKHYgPyAidHJ1ZSIgOiAiZmFsc2UiKTsKKwl9CiAJaWYgKHR5
cGUgPT0gVFlQRV9DT0xPUikgewogCQljaGFyIHZbQ09MT1JfTUFYTEVOXTsKIAkJaWYgKGdpdF9j
b25maWdfY29sb3Iodiwga2V5LCB2YWx1ZSkpCmRpZmYgLS1naXQgYS9tZXJnZXRvb2xzL21lbGQg
Yi9tZXJnZXRvb2xzL21lbGQKaW5kZXggN2EwODQ3MC4uYWFiNGViYiAxMDA2NDQKLS0tIGEvbWVy
Z2V0b29scy9tZWxkCisrKyBiL21lcmdldG9vbHMvbWVsZApAQCAtMywzNCArMyw4NyBAQCBkaWZm
X2NtZCAoKSB7CiB9CiAKIG1lcmdlX2NtZCAoKSB7Ci0JaWYgdGVzdCAteiAiJHttZWxkX2hhc19v
dXRwdXRfb3B0aW9uOitzZXR9IgorCWNoZWNrX21lbGRfZm9yX2ZlYXR1cmVzCisKKwlvcHRpb25f
YXV0b19tZXJnZT0KKwlpZiB0ZXN0ICIkbWVsZF91c2VfYXV0b19tZXJnZV9vcHRpb24iID0gdHJ1
ZQogCXRoZW4KLQkJY2hlY2tfbWVsZF9mb3Jfb3V0cHV0X3ZlcnNpb24KKwkJb3B0aW9uX2F1dG9f
bWVyZ2U9Ii0tYXV0by1tZXJnZSIKIAlmaQogCiAJaWYgdGVzdCAiJG1lbGRfaGFzX291dHB1dF9v
cHRpb24iID0gdHJ1ZQogCXRoZW4KLQkJIiRtZXJnZV90b29sX3BhdGgiIC0tb3V0cHV0PSIkTUVS
R0VEIiBcCisJCSIkbWVyZ2VfdG9vbF9wYXRoIiAkb3B0aW9uX2F1dG9fbWVyZ2UgLS1vdXRwdXQ9
IiRNRVJHRUQiIFwKIAkJCSIkTE9DQUwiICIkQkFTRSIgIiRSRU1PVEUiCiAJZWxzZQotCQkiJG1l
cmdlX3Rvb2xfcGF0aCIgIiRMT0NBTCIgIiRNRVJHRUQiICIkUkVNT1RFIgorCQkiJG1lcmdlX3Rv
b2xfcGF0aCIgJG9wdGlvbl9hdXRvX21lcmdlICIkTE9DQUwiICIkTUVSR0VEIiAiJFJFTU9URSIK
IAlmaQogfQogCi0jIENoZWNrIHdoZXRoZXIgd2Ugc2hvdWxkIHVzZSAnbWVsZCAtLW91dHB1dCA8
ZmlsZT4nCi1jaGVja19tZWxkX2Zvcl9vdXRwdXRfdmVyc2lvbiAoKSB7Ci0JbWVsZF9wYXRoPSIk
KGdpdCBjb25maWcgbWVyZ2V0b29sLm1lbGQucGF0aCkiCi0JbWVsZF9wYXRoPSIke21lbGRfcGF0
aDotbWVsZH0iCisjIEdldCBtZWxkIGhlbHAgbWVzc2FnZQoraW5pdF9tZWxkX2hlbHBfbXNnICgp
IHsKKwlpZiB0ZXN0IC16ICIkbWVsZF9oZWxwX21zZyIKKwl0aGVuCisJCW1lbGRfcGF0aD0iJChn
aXQgY29uZmlnIG1lcmdldG9vbC5tZWxkLnBhdGggfHwgZWNobyBtZWxkKSIKKwkJbWVsZF9oZWxw
X21zZz0kKCIkbWVsZF9wYXRoIiAtLWhlbHAgMj4mMSkKKwlmaQorfQogCi0JaWYgbWVsZF9oYXNf
b3V0cHV0X29wdGlvbj0kKGdpdCBjb25maWcgLS1ib29sIG1lcmdldG9vbC5tZWxkLmhhc091dHB1
dCkKKyMgQ2hlY2sgdGhlIGZlYXR1cmVzIGFuZCBzZXQgZmxhZ3MKK2NoZWNrX21lbGRfZm9yX2Zl
YXR1cmVzICgpIHsKKwkjIENoZWNrIHdoZXRoZXIgd2Ugc2hvdWxkIHVzZSAnbWVsZCAtLW91dHB1
dCA8ZmlsZT4nCisJaWYgdGVzdCAteiAiJG1lbGRfaGFzX291dHB1dF9vcHRpb24iCiAJdGhlbgot
CQk6IHVzZSBjb25maWd1cmVkIHZhbHVlCi0JZWxpZiAiJG1lbGRfcGF0aCIgLS1oZWxwIDI+JjEg
fAotCQlncmVwIC1lICctLW91dHB1dD0nIC1lICdcW09QVElPTlwuXC5cLlxdJyA+L2Rldi9udWxs
CisJCW1lbGRfaGFzX291dHB1dF9vcHRpb249JChnaXQgY29uZmlnIC0tYm9vbCBtZXJnZXRvb2wu
bWVsZC5oYXNPdXRwdXQpCisJCWNhc2UgIiRtZWxkX2hhc19vdXRwdXRfb3B0aW9uIiBpbgorCQl0
cnVlIHwgZmFsc2UpCisJCQk6IHVzZSBjb25maWd1cmVkIHZhbHVlCisJCQk7OworCQkqKQorCQkJ
OiBlbXB0eSBvciBpbnZhbGlkIGNvbmZpZ3VyZWQgdmFsdWUsIGRldGVjdGluZyAiLS1vdXRwdXQi
IGF1dG9tYXRpY2FsbHkKKwkJCWluaXRfbWVsZF9oZWxwX21zZworCisJCQljYXNlICIkbWVsZF9o
ZWxwX21zZyIgaW4KKwkJCSoiLS1vdXRwdXQ9IiogfCAqJ1tPUFRJT04uLi5dJyopCisJCQkJIyBB
bGwgdmVyc2lvbiB0aGF0IGhhcyBbT1BUSU9OLi4uXSBzdXBwb3J0cyAtLW91dHB1dAorCQkJCW1l
bGRfaGFzX291dHB1dF9vcHRpb249dHJ1ZQorCQkJCTs7CisJCQkqKQorCQkJCW1lbGRfaGFzX291
dHB1dF9vcHRpb249ZmFsc2UKKwkJCQk7OworCQkJZXNhYworCQkJOzsKKwkJZXNhYworCWZpCisJ
IyBDaGVjayB3aGV0aGVyIHdlIHNob3VsZCB1c2UgJ21lbGQgLS1hdXRvLW1lcmdlIC4uLicKKwlp
ZiB0ZXN0IC16ICIkbWVsZF91c2VfYXV0b19tZXJnZV9vcHRpb24iCiAJdGhlbgotCQk6IG9sZCBv
bmVzIG1lbnRpb24gLS1vdXRwdXQgYW5kIG5ldyBvbmVzIGp1c3Qgc2F5IE9QVElPTi4uLgotCQlt
ZWxkX2hhc19vdXRwdXRfb3B0aW9uPXRydWUKLQllbHNlCi0JCW1lbGRfaGFzX291dHB1dF9vcHRp
b249ZmFsc2UKKwkJbWVsZF91c2VfYXV0b19tZXJnZV9vcHRpb249JCgKKwkJCWdpdCBjb25maWcg
LS1ib29sLW9yLXN0ciBtZXJnZXRvb2wubWVsZC51c2VBdXRvTWVyZ2UKKwkJKQorCQljYXNlICIk
bWVsZF91c2VfYXV0b19tZXJnZV9vcHRpb24iIGluCisJCXRydWUgfCBmYWxzZSkKKwkJCTogdXNl
IHdlbGwgZm9ybWF0dGVkIGJvb2xlYW4gdmFsdWUKKwkJCTs7CisJCWF1dG8pCisJCQkjIHRlc3Rp
bmcgdGhlICItLWF1dG8tbWVyZ2UiIG9wdGlvbiBvbmx5IGlmIGNvbmZpZyBpcyAiYXV0byIKKwkJ
CWluaXRfbWVsZF9oZWxwX21zZworCisJCQljYXNlICIkbWVsZF9oZWxwX21zZyIgaW4KKwkJCSoi
LS1hdXRvLW1lcmdlIiogfCAqJ1tPUFRJT04uLi5dJyopCisJCQkJbWVsZF91c2VfYXV0b19tZXJn
ZV9vcHRpb249dHJ1ZQorCQkJCTs7CisJCQkqKQorCQkJCW1lbGRfdXNlX2F1dG9fbWVyZ2Vfb3B0
aW9uPWZhbHNlCisJCQkJOzsKKwkJCWVzYWMKKwkJCTs7CisJCSIiKQorCQkJbWVsZF91c2VfYXV0
b19tZXJnZV9vcHRpb249ZmFsc2UKKwkJCTs7CisJCSopCisJCQlkaWUgInVua25vd24gbWVyZ2V0
b29sLm1lbGQudXNlQXV0b01lcmdlOiAkbWVsZF91c2VfYXV0b19tZXJnZV9vcHRpb24iCisJCQk7
OworCQllc2FjCiAJZmkKIH0KLS0gCjIuMi4wCgo=
--0000000000005987f505aed76a61--

@gitgitgadget-git
Copy link

User Lin Sun <lin.sun@zoom.us> has been added to the cc: list.

@dscho
Copy link
Member

dscho commented Sep 9, 2020

@sunlin7 how about force-pushing the update, and then /submiting again? That way, the patch will be sent in the expected format.

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

Thanks.  Will replace.

For those who are watching from the sideline, here is the patch inline.

-- >8 --
From: Lin Sun <lin.sun@zoom.us>
Date: Thu, 7 May 2020 07:31:14 +0800
Subject: [PATCH] Support auto-merge for meld to follow the vim-diff behavior
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make the mergetool used with "meld" backend behave similarly to "vimdiff" by
telling it to auto-merge non-conflicting parts and highlight the conflicting
parts when `mergetool.meld.useAutoMerge` is configured with `true`, or `auto`
for detecting the `--auto-merge` option automatically.

Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Helped-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Lin Sun <lin.sun@zoom.us>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config/mergetool.txt | 10 ++++
 builtin/config.c                   | 17 ++++++
 mergetools/meld                    | 85 ++++++++++++++++++++++++------
 3 files changed, 96 insertions(+), 16 deletions(-)

diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt
index 09ed31dbfa..16a27443a3 100644
--- a/Documentation/config/mergetool.txt
+++ b/Documentation/config/mergetool.txt
@@ -30,6 +30,16 @@ mergetool.meld.hasOutput::
 	to `true` tells Git to unconditionally use the `--output` option,
 	and `false` avoids using `--output`.
 
+mergetool.meld.useAutoMerge::
+	When the `--auto-merge` is given, meld will merge all non-conflicting
+	parts automatically, highlight the conflicting parts and wait for
+	user decision.  Setting `mergetool.meld.useAutoMerge` to `true` tells
+	Git to unconditionally use the `--auto-merge` option with `meld`.
+	Setting this value to `auto` makes git detect whether `--auto-merge`
+	is supported and will only use `--auto-merge` when available.  A
+	value of `false` avoids using `--auto-merge` altogether, and is the
+	default value.
+
 mergetool.keepBackup::
 	After performing a merge, the original file with conflict markers
 	can be saved as a file with a `.orig` extension.  If this variable
diff --git a/builtin/config.c b/builtin/config.c
index ee4aef6a35..7891e070a4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -65,6 +65,7 @@ static int show_scope;
 #define TYPE_PATH		4
 #define TYPE_EXPIRY_DATE	5
 #define TYPE_COLOR		6
+#define TYPE_BOOL_OR_STR	7
 
 #define OPT_CALLBACK_VALUE(s, l, v, h, i) \
 	{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
@@ -94,6 +95,8 @@ static int option_parse_type(const struct option *opt, const char *arg,
 			new_type = TYPE_INT;
 		else if (!strcmp(arg, "bool-or-int"))
 			new_type = TYPE_BOOL_OR_INT;
+		else if (!strcmp(arg, "bool-or-str"))
+			new_type = TYPE_BOOL_OR_STR;
 		else if (!strcmp(arg, "path"))
 			new_type = TYPE_PATH;
 		else if (!strcmp(arg, "expiry-date"))
@@ -149,6 +152,7 @@ static struct option builtin_config_options[] = {
 	OPT_CALLBACK_VALUE(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL),
 	OPT_CALLBACK_VALUE(0, "int", &type, N_("value is decimal number"), TYPE_INT),
 	OPT_CALLBACK_VALUE(0, "bool-or-int", &type, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
+	OPT_CALLBACK_VALUE(0, "bool-or-str", &type, N_("value is --bool or string"), TYPE_BOOL_OR_STR),
 	OPT_CALLBACK_VALUE(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
 	OPT_CALLBACK_VALUE(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
 	OPT_GROUP(N_("Other")),
@@ -250,6 +254,12 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
 				strbuf_addstr(buf, v ? "true" : "false");
 			else
 				strbuf_addf(buf, "%d", v);
+		} else if (type == TYPE_BOOL_OR_STR) {
+			int v = git_parse_maybe_bool(value_);
+			if (v < 0)
+				strbuf_addstr(buf, value_);
+			else
+				strbuf_addstr(buf, v ? "true" : "false");
 		} else if (type == TYPE_PATH) {
 			const char *v;
 			if (git_config_pathname(&v, key_, value_) < 0)
@@ -411,6 +421,13 @@ static char *normalize_value(const char *key, const char *value)
 		else
 			return xstrdup(v ? "true" : "false");
 	}
+	if (type == TYPE_BOOL_OR_STR) {
+		int v = git_parse_maybe_bool(value);
+		if (v < 0)
+			return xstrdup(value);
+		else
+			return xstrdup(v ? "true" : "false");
+	}
 	if (type == TYPE_COLOR) {
 		char v[COLOR_MAXLEN];
 		if (git_config_color(v, key, value))
diff --git a/mergetools/meld b/mergetools/meld
index 7a08470f88..aab4ebb935 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -3,34 +3,87 @@ diff_cmd () {
 }
 
 merge_cmd () {
-	if test -z "${meld_has_output_option:+set}"
+	check_meld_for_features
+
+	option_auto_merge=
+	if test "$meld_use_auto_merge_option" = true
 	then
-		check_meld_for_output_version
+		option_auto_merge="--auto-merge"
 	fi
 
 	if test "$meld_has_output_option" = true
 	then
-		"$merge_tool_path" --output="$MERGED" \
+		"$merge_tool_path" $option_auto_merge --output="$MERGED" \
 			"$LOCAL" "$BASE" "$REMOTE"
 	else
-		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+		"$merge_tool_path" $option_auto_merge "$LOCAL" "$MERGED" "$REMOTE"
 	fi
 }
 
-# Check whether we should use 'meld --output <file>'
-check_meld_for_output_version () {
-	meld_path="$(git config mergetool.meld.path)"
-	meld_path="${meld_path:-meld}"
+# Get meld help message
+init_meld_help_msg () {
+	if test -z "$meld_help_msg"
+	then
+		meld_path="$(git config mergetool.meld.path || echo meld)"
+		meld_help_msg=$("$meld_path" --help 2>&1)
+	fi
+}
 
-	if meld_has_output_option=$(git config --bool mergetool.meld.hasOutput)
+# Check the features and set flags
+check_meld_for_features () {
+	# Check whether we should use 'meld --output <file>'
+	if test -z "$meld_has_output_option"
 	then
-		: use configured value
-	elif "$meld_path" --help 2>&1 |
-		grep -e '--output=' -e '\[OPTION\.\.\.\]' >/dev/null
+		meld_has_output_option=$(git config --bool mergetool.meld.hasOutput)
+		case "$meld_has_output_option" in
+		true | false)
+			: use configured value
+			;;
+		*)
+			: empty or invalid configured value, detecting "--output" automatically
+			init_meld_help_msg
+
+			case "$meld_help_msg" in
+			*"--output="* | *'[OPTION...]'*)
+				# All version that has [OPTION...] supports --output
+				meld_has_output_option=true
+				;;
+			*)
+				meld_has_output_option=false
+				;;
+			esac
+			;;
+		esac
+	fi
+	# Check whether we should use 'meld --auto-merge ...'
+	if test -z "$meld_use_auto_merge_option"
 	then
-		: old ones mention --output and new ones just say OPTION...
-		meld_has_output_option=true
-	else
-		meld_has_output_option=false
+		meld_use_auto_merge_option=$(
+			git config --bool-or-str mergetool.meld.useAutoMerge
+		)
+		case "$meld_use_auto_merge_option" in
+		true | false)
+			: use well formatted boolean value
+			;;
+		auto)
+			# testing the "--auto-merge" option only if config is "auto"
+			init_meld_help_msg
+
+			case "$meld_help_msg" in
+			*"--auto-merge"* | *'[OPTION...]'*)
+				meld_use_auto_merge_option=true
+				;;
+			*)
+				meld_use_auto_merge_option=false
+				;;
+			esac
+			;;
+		"")
+			meld_use_auto_merge_option=false
+			;;
+		*)
+			die "unknown mergetool.meld.useAutoMerge: $meld_use_auto_merge_option"
+			;;
+		esac
 	fi
 }
-- 
2.28.0-558-g7a0184fd7b

@gitgitgadget-git
Copy link

This patch series was integrated into seen via a98dad7.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via ca35cc9.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via afa52bd.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Sep 11, 2020

@dscho thank you for reminding, @gitster already recieved the patch, I'm not sure should I do /submit in this page again.

@dscho
Copy link
Member

dscho commented Sep 12, 2020

Yes, the new iteration was picked up, but the downside of not using GitGitGadget is that there is no tagged version of v2, nor a range-diff, i.e. it makes it harder than necessary for people who want to follow this patch's evolution.

OTOH a /submit now would probably annoy other people, as the new iteration had already be sent.

But then, should yet another iteration be necessary, using GitGitGadget would really confuse people, as that would be sent as "another" second iteration.

I guess that /submiting at this stage would be about equally as bad an option as not /submiting.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Sep 12, 2020

@dscho thank you for comment, I'll submit for complete GitGitGadget flow.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Sep 12, 2020

/submit

@gitgitgadget-git
Copy link

Submitted as pull.781.v18.git.git.1599895268433.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-git-781/sunlin7/master-v18

To fetch this version to local tag pr-git-781/sunlin7/master-v18:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-git-781/sunlin7/master-v18

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

"sunlin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Lin Sun <lin.sun@zoom.us>
>
> Make the mergetool used with "meld" backend behave similarly to "vimdiff" by
> telling it to auto-merge non-conflicting parts and highlight the conflicting
> parts when `mergetool.meld.useAutoMerge` is configured with `true`, or `auto`
> for detecting the `--auto-merge` option automatically.
>
> Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> Helped-by: David Aguilar <davvid@gmail.com>
> Signed-off-by: Lin Sun <lin.sun@zoom.us>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>     Enable auto-merge for meld to follow the vimdiff beharior
>     
>     Hi, the mergetool "meld" does NOT merge the no-conflict changes, while
>     the mergetool "vimdiff" will merge the no-conflict changes and highlight
>     the conflict parts. This patch will make the mergetool "meld" similar to
>     "vimdiff", auto-merge the no-conflict changes, highlight conflict parts.

Thanks.

@gitgitgadget-git
Copy link

On the Git mailing list, Lin Sun wrote (reply to this):

Hi Junio,

> conflict …
It’s my typo. Thank you for fixing it.

Best Regards
Lin Sun

@gitgitgadget-git
Copy link

This patch series was integrated into seen via 65ffb34.

@gitgitgadget-git
Copy link

This patch series was integrated into next via 01985a6.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via a3728d7.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via 0e16dc0.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via 1a4e36a.

@gitgitgadget-git
Copy link

This patch series was integrated into seen via 4aff18a.

@gitgitgadget-git
Copy link

This patch series was integrated into next via 4aff18a.

@gitgitgadget-git
Copy link

This patch series was integrated into master via 4aff18a.

@gitgitgadget-git
Copy link

Closed via 4aff18a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants