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

[Junio C Hamano <gitster@pobox.com>]

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

> +		meld_use_auto_merge_option=$(git config mergetool.meld.useAutoMerge)
> +		case "$meld_use_auto_merge_option" in
> +		"")
> +			meld_use_auto_merge_option=false
> +			;;

I somehow thought that I already pointed out that this is wrong,
didn't I?  You cannot tell between a "[section] var" (which is
"true") and not having any "[section] var = val" (which I think you
are trying to treat as "not configured---do not use") from the
output of "git config section.var".

Perhaps our mails crossed?

> +		[Tt]ure|TRUE)
> +			meld_use_auto_merge_option=true
> +			;;
> +		[Ff]alse|FALSE)
> +			meld_use_auto_merge_option=false
> +			;;

These are probably premature optimizations.

> +		[Aa]uto|AUTO)

Sigh.  I somehow thought that I already said we shouldn't do this
"aCCEpt AnY CaSES" unless all other variables that take 'auto' take
it case insensitively.

> +			# testing the "--auto-merge" option only if config is "auto"
> +			init_meld_help_msg
> +
> +			case "$meld_help_msg" in
> +			*"--auto-merge"*)
> +				meld_use_auto_merge_option=true
> +				;;
> +			*)
> +				meld_use_auto_merge_option=false
> +				;;
> +			esac
> +			;;
> +		*)
> +			# try detect boolean for 'on'||'yes'||numberic value
> +			bool_value=$(git config --bool mergetool.meld.useAutoMerge 2>/dev/null)
> +			if test -n "$bool_value"
> +			then
> +				meld_use_auto_merge_option="$bool_value"
> +			else
> +				meld_use_auto_merge_option=false

I think this case (i.e. set to a non-bool value, and we do not
recognise because it is not 'auto') should be flagged as an error,
instead of treated as a silent "do not use", as it would leave the
user scratching his or her head without realizing that there is a
typo in the configuration file.

> +			fi
> +			;;
> +		esac
>  	fi
>  }
>
> base-commit: 07d8ea56f2ecb64b75b92264770c0a664231ce17