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

From: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Date: Jul 7, 2020, 15:31 UTC
Message-ID: <20200707153155.GC7003@danh.dev>
In-Reply-To: <pull.781.v11.git.git.1594106024078.gitgitgadget@gmail.com>

On 2020-07-07 07:13:43+0000, sunlin via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 10 quoted lines

> -# 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)

I haven't thought about this until now. `$meld_path` should be quoted.

	meld_help_msg="$("$meld_path" --help 2>&1)"

Either that or our Windows friends will complain soon:

	C:/Program Files/Meld/meld --help

(I don't know where meld will be installed by default in Window exactly, above command is just a guess)

Show 24 quoted lines

> +	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

Good comments :)

Show 37 quoted lines

> +				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
> +		if meld_use_auto_merge_option=$( \
> +			git config --bool mergetool.meld.useAutoMerge 2>/dev/null)
> +		then
> +			: use configured value
> +		else
> +			meld_use_auto_merge_option=$(git config mergetool.meld.useAutoMerge)
> +			if test "$meld_use_auto_merge_option" = "auto"
> +			then
> +				# 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
> +			else
> +				meld_use_auto_merge_option=false

If I read other emails correctly (which I usually don't), I think we would want to bail out in this case.

Show 8 quoted lines

> +			fi
> +		fi
>  	fi
>  }
> 
> base-commit: 07d8ea56f2ecb64b75b92264770c0a664231ce17
> -- 
> gitgitgadget

-- 
Danh