From: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
To: sunlin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, sunlin <sunlin7@yahoo.com>,
"lin.sun" <lin.sun@zoom.us>
Subject: Re: [PATCH v4] Enable auto-merge for meld to follow the vim-diff beharior
Date: Tue, 30 Jun 2020 23:23:33 +0700 [thread overview]
Message-ID: <20200630162333.GA15821@danh.dev> (raw)
In-Reply-To: <pull.781.v4.git.git.1593516397380.gitgitgadget@gmail.com>
On 2020-06-30 11:26:37+0000, sunlin via GitGitGadget <gitgitgadget@gmail.com> wrote:
> From: "lin.sun" <lin.sun@zoom.us>
This part needs to be fixed, aka, use your real name here :).
>
> 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.
>
> 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-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-781/sunlin7/master-v4
> Pull-Request: https://github.com/git/git/pull/781
>
> Range-diff vs v3:
>
> 1: 3b70fd0bfc ! 1: 93ae3ec011 Enable auto-merge for meld to follow the vim-diff beharior
> @@ Metadata
> ## Commit message ##
> Enable auto-merge for meld to follow the vim-diff beharior
>
> - 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.
> + 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.
>
> - Signed-off-by: Lin Sun <lin.sun@zoom.us>
> + Signed-off-by: lin.sun <lin.sun@zoom.us>
>
> ## 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
> +@@ mergetools/meld: diff_cmd () {
> + }
> +
> + merge_cmd () {
> +- if test -z "${meld_has_output_option:+set}"
> ++ check_meld_for_features
> +
> + option_auto_merge=
> + if test "$meld_has_auto_merge_option" = true
> -+ then
> + then
> +- check_meld_for_output_version
> + option_auto_merge="--auto-merge"
> -+ fi
> + fi
>
> if test "$meld_has_output_option" = true
> then
> @@ mergetools/meld: merge_cmd () {
> 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}"
> +-# 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
> ++get_meld_help_msg () {
> ++ meld_path="$(git config mergetool.meld.path || echo meld)"
> ++ $meld_path --help 2>&1
> ++}
> +
> +- 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:+set}"
> + 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)
> ++ if test "$meld_has_output_option" = true -o \
> ++ "$meld_has_output_option" = false
> ++ then
> ++ : use configured value
> ++ else # treat meld_has_output_option as "auto"
> ++ if test -z "$meld_help_msg"
> ++ then
> ++ meld_help_msg="$(get_meld_help_msg)"
> ++ fi
> +
> -+ 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
> ++ if echo "$meld_help_msg" |
> ++ grep -e '--output=' -e '\[OPTION\.\.\.\]' >/dev/null
> ++ then
> ++ : old ones mention --output and new ones just say OPTION...
> ++ meld_has_output_option=true
> ++ else
> ++ meld_has_output_option=false
> ++ fi
> ++ fi
> + fi
> -+}
> ++ # Check whether we should use 'meld --auto-merge ...'
> ++ if test -z "${meld_has_auto_merge_option:+set}"
> + then
> +- : old ones mention --output and new ones just say OPTION...
> +- meld_has_output_option=true
> +- else
> +- meld_has_output_option=false
> ++ meld_has_auto_merge_option=$(git config --bool mergetool.meld.hasAutoMerge)
> ++ if test "$meld_has_auto_merge_option" = true -o \
> ++ "$meld_has_auto_merge_option" = false
> ++ then
> ++ : use configured value
> ++ else # treat meld_has_auto_merge_option as "auto"
> ++ if test -z "$meld_help_msg"
> ++ then
> ++ meld_help_msg="$(get_meld_help_msg)"
> ++ fi
> ++
> ++ if echo "$meld_help_msg" | grep -e '--auto-merge' >/dev/null
> ++ then
> ++ meld_has_auto_merge_option=true
> ++ else
> ++ meld_has_auto_merge_option=false
> ++ fi
> ++ fi
> + fi
> + }
>
>
> mergetools/meld | 73 ++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 57 insertions(+), 16 deletions(-)
>
> diff --git a/mergetools/meld b/mergetools/meld
> index 7a08470f88..1b92771228 100644
> --- a/mergetools/meld
> +++ b/mergetools/meld
> @@ -3,34 +3,75 @@ diff_cmd () {
> }
>
> merge_cmd () {
> - if test -z "${meld_has_output_option:+set}"
> + check_meld_for_features
> +
> + option_auto_merge=
> + if test "$meld_has_auto_merge_option" = true
We probably want "meld_want_auto_merge"?
Someone may use meld that has --auto-merge but don't want to use it.
> 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
> +get_meld_help_msg () {
> + meld_path="$(git config mergetool.meld.path || echo meld)"
> + $meld_path --help 2>&1
We always use TAB for indentation.
> +}
>
> - 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:+set}"
> 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)
> + if test "$meld_has_output_option" = true -o \
> + "$meld_has_output_option" = false
> + then
> + : use configured value
> + else # treat meld_has_output_option as "auto"
Please don't put your comment like this,
it's too far away from code,
reader may miss that.
> + if test -z "$meld_help_msg"
> + then
> + meld_help_msg="$(get_meld_help_msg)"
> + fi
> +
> + if echo "$meld_help_msg" |
> + grep -e '--output=' -e '\[OPTION\.\.\.\]' >/dev/null
I probably write like this to save a pipe, a fork and an exec:
case "$meld_help_msg" in
*"[OPTION"???"]"*)
meld_has_output_option=true ;;
*)
meld_has_output_option=false ;;
esac
If you don't like the case ... esac,
I prefer "grep -q" in this case,
I didn't check how much portable it's.
Don't forget to hide the error with 2>/dev/null (or in you case 2>&1)
> + then
> + : old ones mention --output and new ones just say OPTION...
> + meld_has_output_option=true
> + else
> + meld_has_output_option=false
> + fi
> + fi
> + fi
> + # Check whether we should use 'meld --auto-merge ...'
> + if test -z "${meld_has_auto_merge_option:+set}"
> then
> - : old ones mention --output and new ones just say OPTION...
> - meld_has_output_option=true
I think something was forgotten here?
> - else
> - meld_has_output_option=false
> + meld_has_auto_merge_option=$(git config --bool mergetool.meld.hasAutoMerge)
> + if test "$meld_has_auto_merge_option" = true -o \
> + "$meld_has_auto_merge_option" = false
> + then
> + : use configured value
> + else # treat meld_has_auto_merge_option as "auto"
> + if test -z "$meld_help_msg"
> + then
> + meld_help_msg="$(get_meld_help_msg)"
> + fi
> +
> + if echo "$meld_help_msg" | grep -e '--auto-merge' >/dev/null
And I'll use the same "case ... esac" above for this one :)
> + then
> + meld_has_auto_merge_option=true
> + else
> + meld_has_auto_merge_option=false
> + fi
> + fi
> fi
> }
>
> base-commit: 07d8ea56f2ecb64b75b92264770c0a664231ce17
> --
> gitgitgadget
--
Danh
next prev parent reply other threads:[~2020-06-30 16:23 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-08 1:25 [PATCH] Enable auto-merge for meld to follow the vim-diff beharior sunlin via GitGitGadget
2020-06-08 9:49 ` Pratyush Yadav
2020-06-09 3:19 ` [PATCH v2] " sunlin via GitGitGadget
2020-06-29 7:07 ` [PATCH v3] " sunlin via GitGitGadget
2020-06-29 12:32 ` Fwd: " Git Gadget
2020-06-30 0:06 ` Junio C Hamano
2020-06-30 7:42 ` David Aguilar
2020-06-30 11:25 ` lin.sun
2020-06-30 11:37 ` lin.sun
2020-06-30 15:51 ` Junio C Hamano
2020-06-30 11:26 ` [PATCH v4] " sunlin via GitGitGadget
2020-06-30 16:23 ` Đoàn Trần Công Danh [this message]
2020-06-30 23:01 ` Đoàn Trần Công Danh
2020-07-01 7:06 ` [PATCH v5] " sunlin via GitGitGadget
2020-07-01 7:23 ` lin.sun
2020-07-01 18:19 ` David Aguilar
2020-07-01 14:17 ` Đoàn Trần Công Danh
2020-07-01 15:32 ` lin.sun
2020-07-01 22:02 ` lin.sun
2020-07-01 23:06 ` Đoàn Trần Công Danh
2020-07-01 19:51 ` Junio C Hamano
2020-07-02 0:20 ` lin.sun
2020-07-02 0:44 ` [PATCH v6] Support auto-merge for meld to follow the vim-diff behavior sunlin via GitGitGadget
2020-07-02 2:35 ` lin.sun
2020-07-03 1:50 ` Junio C Hamano
2020-07-03 3:53 ` lin.sun
2020-07-03 15:58 ` Đoàn Trần Công Danh
2020-07-06 6:23 ` Junio C Hamano
2020-07-03 3:26 ` [PATCH v7] " sunlin via GitGitGadget
2020-07-03 4:50 ` Junio C Hamano
2020-07-04 1:18 ` lin.sun
2020-07-06 2:36 ` lin.sun
2020-07-04 1:16 ` [PATCH v8] " sunlin via GitGitGadget
2020-07-06 2:27 ` [PATCH v9] " sunlin via GitGitGadget
2020-07-06 22:31 ` Junio C Hamano
2020-07-07 6:34 ` lin.sun
2020-07-07 16:43 ` Junio C Hamano
2020-07-08 1:20 ` lin.sun
2020-07-08 1:51 ` Junio C Hamano
2020-07-07 6:17 ` [PATCH v10] " sunlin via GitGitGadget
2020-07-07 6:25 ` Junio C Hamano
2020-07-07 6:38 ` lin.sun
2020-07-07 6:44 ` lin.sun
2020-07-07 7:13 ` [PATCH v11] " sunlin via GitGitGadget
2020-07-07 15:31 ` Đoàn Trần Công Danh
2020-07-08 0:57 ` lin.sun
2020-07-08 3:25 ` [PATCH v12] " sunlin via GitGitGadget
2020-07-08 3:31 ` lin.sun
2020-07-08 15:42 ` Đoàn Trần Công Danh
2020-07-08 15:47 ` lin.sun
2020-07-09 0:35 ` [PATCH v13] " sunlin via GitGitGadget
2020-07-09 0:39 ` lin.sun
2020-07-09 2:42 ` Junio C Hamano
2020-07-09 2:56 ` Junio C Hamano
2020-07-09 3:24 ` lin.sun
2020-07-09 4:49 ` Junio C Hamano
2020-07-09 5:31 ` Junio C Hamano
2020-07-12 14:07 ` lin.sun
2020-07-12 23:38 ` lin.sun
2020-07-09 4:28 ` [PATCH v14] " sunlin via GitGitGadget
2020-07-12 8:39 ` [PATCH v15] " sunlin via GitGitGadget
2020-07-12 9:08 ` [PATCH v16] " sunlin via GitGitGadget
2020-07-12 18:04 ` Junio C Hamano
2020-07-12 23:26 ` lin.sun
2020-07-13 5:14 ` Junio C Hamano
2020-07-13 6:58 ` lin.sun
2020-07-12 23:32 ` [PATCH v17] " sunlin via GitGitGadget
2020-07-24 0:58 ` Junio C Hamano
2020-09-03 21:48 ` Junio C Hamano
[not found] ` <C35AC799-B4F6-4A5E-92FA-21065310B379@hxcore.ol>
2020-09-09 1:31 ` Lin Sun
2020-09-09 20:43 ` Junio C Hamano
2020-09-12 7:21 ` [PATCH v18] " sunlin via GitGitGadget
2020-09-14 20:07 ` Junio C Hamano
2020-09-15 0:55 ` Lin Sun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200630162333.GA15821@danh.dev \
--to=congdanhqx@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=lin.sun@zoom.us \
--cc=sunlin7@yahoo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.