Re: [PATCH v12] Support auto-merge for meld to follow the vim-diff behavior

From: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Date: Jul 8, 2020, 15:42 UTC
Message-ID: <20200708154218.GB18432@danh.dev>
In-Reply-To: <pull.781.v12.git.git.1594178716840.gitgitgadget@gmail.com>

On 2020-07-08 03:25:16+0000, sunlin via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 18 quoted lines

> +	# 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...]')

The "*'[OPTION...]'*" part sneaks into here in v11, And this version drops the last "*", which makes it:

- less correct if all versions that has "[OPTION...]" supports
  --auto-merge, which is unlikely; and
- less incorrect (still not correct enough) if some versions
  that has "[OPTION...]" doesn't support --auto-merge,
  which is more likely.

If we could trace all versions of meld and confirms the former condition, I think it's better to add back the missing "*"

Otherwise, it's safer choice to remove "|*'[OPTION...]" here.

I'm not using meld, so, if someone asked me, I would go with the latter.

-- 
Danh