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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2020, 05:31 UTC
Message-ID: <xmqq8sftz23v.fsf@gitster.c.googlers.com>
In-Reply-To: <xmqqd055z40h.fsf@gitster.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 12 quoted lines

>>>> +			else
>>>> +				meld_use_auto_merge_option=false
>>>Why? Shoudln't we loudly complain to let the user know of a misspelt value in the configuration?
>>
>> The command line `git config --bool ...` without "2>&/dev/null" will print error message, just passthrough to user.
>> $ git config mergetool.meld.useAutoMerge hello 
>> $ git mergetool --tool=meld 
>> fatal: bad numeric config value 'hello' for 'mergetool.meld.useautomerge' in .git/config: invalid unit
>
> Yes, but you do not exit(1) here, so the user will keep going
> without having a chance to stop, think and correct the misspelt
> value in the configuration file, no?

Hmph, I've been forgetting an important case you are handling well, which is that the user does not have the variable defined anywhere in the configuration. "git config --bool var.ia.ble" fails silently in such a case, and you do want to declare that the user does not want to use the "--auto-merge" in that case, which is what this silent else clause does very well.

So, let's leave this part as-is.

Thanks, I'll push out a new iteration of today's integration before going to bed.

Show 6 quoted lines

>
>>
>> I'll upload the changes soon.
>>
>> Regards
>> Lin