Re: so/diff-merges-more (was Re: What's cooking in git.git (Feb 2023, #01; Thu, 2))

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 2, 2023, 16:15 UTC
Message-ID: <xmqqr0u7ku3j.fsf@gitster.g>
In-Reply-To: <kl6lh6v43s4g.fsf@chooglen-macbookpro.roam.corp.google.com>

Glen Choo <chooglen@google.com> writes:

Show 7 quoted lines

>> Finally, event without "log.diffMerges-m-imply-p", the rest of the
>> series still makes sense, so if the conclusion is to remove it, let's
>> still merge the rest, please! Let me know, and I'll then remove the
>> patch and will change documentation accordingly.
>
> Oops. Sorry, I missed this the first time I read this message. This
> alone should have warranted a response.

Hmph. I agreed with you that the last step to add the configuration would not make sense unless we are planning to break users later, but I have been under the impression that the remainder were OK clean-ups. Perhaps it is mostly because I read them so long ago and forgot the details X-<.

Show 21 quoted lines

> I'm not convinced that the series makes sense without
> "log.diffMerges-m-imply-p":
>
> * The main patch is
>
>     diff-merges: implement [no-]hide option and log.diffMergesHide config
>
>   which gives us a way to redefine "-m" as "--diff-merges=hide
>   --diff-merges=on". However, we haven't seen any compelling reasons to
>   use --diff-merges=hide [1]. I'm also fairly convinced that if we go
>   back in time, "-m" wouldn't have the semantics of 'do nothing unless
>   -p is also given', and I don't think we should immortalize a mistake
>   by giving it an explicit option.
>
>   All the other patches in their current form are dependent on this
>   patch going in.
>
> * diff-merges: support list of values for --diff-merges
>
>   This only makes sense if we want to accept multiple values, which we
>   don't without the main patch.

Now you mention it (and show example in the previous bullet point), I have to agree that we would not want this, not because we do not want to have --diff-merges with multiple values, but because it introduces an odd-man-out option that is not "last one wins" that is not used anywhere else in the UI.

No response does not necessarily mean an agreement, but it indeed helped in this case to be explicit.

Thanks for the clarification.