Re: [PATCH] mergetool: do not enable hideResolved by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Nieder <jrnieder@xxxxxxxxx> writes:

> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 911470a5b2..f751d9cfe2 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -358,13 +358,8 @@ merge_file () {
>  		    enabled=false
>  		fi
>  	else
> -		# The user does not have a preference. Ask the tool.
> -		if hide_resolved_enabled
> -		then
> -		    enabled=true
> -		else
> -		    enabled=false
> -		fi
> +		# The user does not have a preference. Default to disabled.
> +		enabled=false

OK.  So the logic used to be

 - If the user has preference for a specific backend, use it;

 - If the user says the feature is unwanted, that is final;

 - If the user says it generally is OK to use the feature, let each
   backend set the preference;

 - If there is no preference, let each backend set the preference.

As we want to disable the feature for any backend when the user does
not explicitly say the feature is wanted (either in general, or for
a specific backend), the change in the above hunk is exactly want we
want to see.

Looking good.  Let's not revert the series and disable by default.

Should I expect an updated log message, though?  What was in the
proposed log message sounded more unsubstantiated complaint than
giving readable reasons why the feature is unwanted, but both the
response by Seth and your response to Seth's response had material
that made it more convincing why we would want to disable this by
default, e.g. "with little to no explanation", "We don't have a way
to communicate to the end-user" (both by Seth), "when ... didn't end
up lining up the files correctly", "no way to visually distinguish"
(yours) are all good ingredients to explain why this feature is
prone to subtly and silently give wrong information to the
end-users.

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux