Re: [PATCH v2] wt-status: use rename settings from init_diff_ui_defaults

[Elijah Newren <newren@gmail.com>]

On Tue, May 1, 2018 at 4:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
>> I'm not certain what the default should be, but I do believe that it
>> should be consistent between these commands.  I lean towards
>> considering break detection being on by default a good thing, but
>> there are some interesting issues to address:
>>   - there is no knob to adjust break detection for status, only for
>> diff/log/show/etc.
>>   - folks have a knob to turn break detection on (for diff/log/show),
>> but not one to turn it off
>>   - for status, break detection makes no sense if rename detection is off.
>>   - for diff/log/show, break detection provides almost no value if
>> rename detection is off (only a dissimilarity index), suggesting that
>> if user turns rename detection off and doesn't explicitly ask for
>> break detection, then it's a waste of time to have it be on
>>   - merge-recursive would break horribly right now if someone turned
>> break detection on for it.  Turning it on might be a good long term
>> goal, but it's not an easy change.
>
> Many of the issues in the above list are surmountable.  A new option
> could be added to "status" to enable break or "diff" family to
> disable it if we really wanted to.  A new "rewritten" state can be
> added alongside with "modified" to "status" output.
>
> A more serious issue around "-B" is this one:
>
>     https://public-inbox.org/git/xmqqegqaahnh.fsf@gitster.dls.corp.google.com/
>
> Even though the message is back from 2015 and asks users not to use
> -B/-M together for anything critical "for now", the issue has not
> been resolved and the same bug remains with us in the current code.
>
> In the longer term, I suspect that it might make sense to have an
> option to let users choose among "I do not want to have anything to
> do with -B", "I always want -B when I ask for -M" and "I always want
> -B whether I ask for -M".  But unfortunately the latter two with the
> current codebase is an unacceptably risky/broken choice.

Very interesting; I didn't know that break detection and rename
detection were unsafe to use together.

I also just realized that in addition to status being inconsistent
with diff/log/show, it was also inconsistent with itself -- it handles
staged and unstaged changes differently.
(wt_status_collect_changes_worktree() had different settings for break
detection than wt_status_collect_changes_index().)

Eckhard, can you add some comments to your commit message mentioning
the email pointed to by Junio about break detection and rename
detection being unsafe to use together, as well as the inconsistencies
in break detection between different commands?  That may help future
readers understand why break detection was turned off for
wt_status_collect_changes_index().