| Message ID | pull.781.v17.git.git.1594596738929.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v17] Support auto-merge for meld to follow the vim-diff behavior | expand |
"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. This version seems to revert the bool-or-str change that would become useful with a bit more polishing, with docs and tests. Intended? It also seems to repeat many issues that have been pointed out during the reviews of previous revisions. I'd rather not have to repeat pointing them out all over again, obviously. I have queued v16 (the one with the beginning of bool-or-str) with minimum fixups for issues I pointed out in the review of that revision queued on top of it as SQUASH??? commit. Please find it in the 'seen' branch. Also, Subject: Support auto-merge for meld to follow the vim-diff behavior needs fixing. cf. https://git-scm.com/docs/SubmittingPatches#describe-changes Two tricks to pick a good title are: - Read a pageful or two of "git shortlog --no-merges" output to get accustomed to the general pattern in the entire project. It would become clear why titles with "area:" prefix help the patches with them easier to locate. - Read a pageful of "git shortlog --no-merges -- mergetools" (i.e. the same but limited to the files you are touching) to see how the changes that contributed over time to build the subsystem are called, so that the new patches can fit in the pattern. I think something along the lines of Subject: [PATCH] mergetool/meld: optionally allow --auto-merge behaviour or something would fit well. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > I have queued v16 (the one with the beginning of bool-or-str) with > minimum fixups for issues I pointed out in the review of that > revision queued on top of it as SQUASH??? commit. Please find it in > the 'seen' branch. > > Also, > > Subject: Support auto-merge for meld to follow the vim-diff behavior > > needs fixing. > ... Anything new on this topic? No rush, but I'd hate to see a basically good topic to be left in the stalled state too long. Thanks.
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 “send box”, I’m not sure if the mail already
sent or not.
So I sent this mail with
“0001-Support-auto-merge-for-meld-to-follow-the-vim-diff-b.patch” 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
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 }
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/mergetools/meld b/mergetools/meld index 7a08470f88..d95b4ee630 100644 --- a/mergetools/meld +++ b/mergetools/meld @@ -3,34 +3,89 @@ 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 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 + ;; + *) + # try other boolean value with git + if meld_use_auto_merge_option=$( + git config --bool mergetool.meld.useAutoMerge) + then + : use normalized boolean value + else + meld_use_auto_merge_option=false + fi + ;; + esac fi }