Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 17, 2016, 01:30 UTC
- Message-ID
- <xmqq7f6zqr3i.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20161217005442.5866-1-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
Show 18 quoted lines
> 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.
Show 14 quoted lines
> 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;
> }