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

From: Junio C Hamano <gitster@pobox.com>
To: <lin.sun@zoom.us>
Cc: "'sunlin via GitGitGadget'" <gitgitgadget@gmail.com>,
	<git@vger.kernel.org>, "'sunlin'" <sunlin7@yahoo.com>
Subject: Re: [PATCH v9] Support auto-merge for meld to follow the vim-diff behavior
Date: Tue, 07 Jul 2020 18:51:42 -0700	[thread overview]
Message-ID: <xmqqeepm3hap.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <04e201d654c6$016bfb40$0443f1c0$@zoom.us> (lin sun's message of "Wed, 8 Jul 2020 09:20:40 +0800")

<lin.sun@zoom.us> writes:

> if meld_use_auto_merge_option =` git config --bool ...`
> then
> 	# success, maybe "(empty)"/true/false
> 	if test -z "$ meld_use_auto_merge_option"
> 	then
> 		meld_use_auto_merge_option=false
> 	fi

The inner test does not make any sense to me; I doubt that you can
get an empty output from a "git config --bool" that succeeds.

     $ cat >config <<\EOF
     [x] a
         b=true
         c=1
         d=false
         e=no
	 f=bogus
     EOF
     $ for i in a b c d e f
       do
         if v=$(git config --file=config --bool x.$i 2>/dev/null)
         then
           echo "succeeds (x.$i) => '$v'"
         else
           echo "fails (x.$i) => '$v'"
         fi
       done

You should see

succeeds (x.a) => 'true'
succeeds (x.b) => 'true'
succeeds (x.c) => 'true'
succeeds (x.d) => 'false'
succeeds (x.e) => 'false'
fails (x.f) => ''

if you run the above.  The most interesting is how all boolean
values are normalized to true and false regardless of how they are
originally spelled.

> else
> 	# failed, the option is not empty or Boolean
> 	if test "auto" = ` git config `
> 	then
> 		# detect the "--auto-merge" option
> 	else
> 		meld_use_auto_merge_option=false

I think review by Đoàn pointed out that we would want to warn/fail
loudly in this "else" case, as it is likely a misspelt configuration
value?

To recap, I think something along the following lines would be the
most readable and acceptably efficient with a short-cut for the
common cases.

    var=mergetool.meld.whatever
    val=$(git config $var)
    case "$val" in
    true | false)
	# we can take a short-cut without asking config --bool
        ... use $val ...
	;;
    auto)
	... auto detect ...
	;;
    *)
	if val=$(git config --bool $var 2>/dev/null)
	then
		# succeeded, so val must be a normalized boolean
		... use $val ...
	else
		die cannot parse $var
	fi
    easc

Thanks.