Re: [PATCH] diff: prefer indent heuristic over compaction heuristic

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

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> The current configuration code for enabling experimental heuristics
> prefers the last-set heuristic in the configuration. However, it is not
> necessarily easy to see what order the configuration will be read. This
> means that it is possible for a user to have accidentally enabled both
> heuristics, and end up only enabling the older compaction heuristic.
>
> Modify the code so that we do not clear the other heuristic when we set
> each heuristic enabled. Then, during diff_setup() when we check the
> configuration, we will first check the newer indent heuristic. This
> ensures that we only enable the newer heuristic if both have been
> enabled.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
>  diff.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)

Although I do not think we should spend too much braincycles on this
one (we should rather just removing the older one soonish), I think
this patch is going in a wrong direction.  I agree that "the last
one wins" is a bit hard to see (until you check with "git config -l"
perhaps) but it at least is predictable.  With this patch, you need
to KNOW that indent wins over compaction, perhaps by knowing the
order they were developed, which demands a lot more from the users.

We probably should just keep one and remove the other.

> diff --git a/diff.c b/diff.c
> index ec8728362dae..48a5b2797e3d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -223,16 +223,10 @@ void init_diff_ui_defaults(void)
>  
>  int git_diff_heuristic_config(const char *var, const char *value, void *cb)
>  {
> +	if (!strcmp(var, "diff.indentheuristic"))
>  		diff_indent_heuristic = git_config_bool(var, value);
> +	if (!strcmp(var, "diff.compactionheuristic"))
>  		diff_compaction_heuristic = git_config_bool(var, value);
>  	return 0;
>  }