From: Junio C Hamano Date: Sat, 17 Dec 2016 01:30:57 GMT Subject: Re: [PATCH] diff: prefer indent heuristic over compaction heuristic Message-ID: In-Reply-To: <20161217005442.5866-1-jacob.e.keller@intel.com> Jacob Keller writes: > From: Jacob Keller > > 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 > --- > 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; > }