On 11/01/2016 10:38 PM, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> If it involves renaming and swapping, however, we may be talking >> about a change that we cannot deliver with confidence in less than a >> week before -rc1, which sounds, eh, suboptimal. >> >> FWIW, here is how the removal of compaction without renaming looks >> like. > > And here is _with_ renaming. I think "compaction heuristics" is a > much better phrase to call "heuristics used during the diff hunk > compaction process" without specifying how that heuristics work > (like taking hints in the indentation levels). If we are to retire > one while keeping the other, compaction-heuristics should be the > name we give and keep for the surviving one. Personally, I'm not a fan of the name "compaction heuristics". The name *seems* to make sense because it affects the behavior of a function called `xdl_change_compact()`. But that means nothing to end users. The option doesn't affect how the diff is "compacted" in the usual sense of the word; the slider optimization usually doesn't change the number of lines in the diff at all [1]. Moreover, if we ever want to add another heuristic (for example, imagine a language-aware algorithm that is based on parsing the file), we would want a different name for that option, at least temporarily. From that point of view, it makes a little bit of sense for the name of the option to hint at the particular heuristic being used. That being said, I don't think it is a big deal either way, and I can live with either name. I rather hope that this option will become the default soon, and hopefully after that hardly anybody will need to learn its name. Regarding making it the default: given that it is presumably too late in this release cycle to make this option the default behavior, I suggest that it be made the default early in the next release cycle so that it gets a lot of testing, and people have enough time to voice any objections. Regarding your patches themselves, once the old compaction heuristic is removed (with or without renaming), you can also get rid of the `blank_lines` local variable in `xdl_change_compact()`, and also the function `is_blank_line()` in the same source file. Michael [1] The only exceptions are when it causes context lines for adjacent diff hunks to join/split, or makes the context lines for a diff hunk extend past the beginning/end of the file. But these are uninteresting side-effects; it doesn't *try* to do or avoid either of these things.