Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views

[Felipe Contreras <felipe.contreras@gmail.com>]

David Aguilar wrote:
> On Sat, Dec 19, 2020 at 11:53 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > David Aguilar wrote:
> > > On Sat, Dec 19, 2020 at 12:59 PM Felipe Contreras
> > > <felipe.contreras@gmail.com> wrote:
> > > > They don't have to rely on that failure, they can just turn off
> > > > mergetool.automerge.
> > > >
> > > >
> > > > But fine. Let's the perfect be the enemy of the good. That seems wise.
> > >
> > > FWIW I'm in favor of having per-tool configuration precisely for
> > > custom mergetools that do things with custom file formats and benefit
> > > from having all of LOCAL REMOTE and BASE.
> >
> > That's a preference, not an argument.
> 
> Lol, that's why I said "in favor".  But, since you asked, it turns out
> that my preferences have a stronger weight than yours.
> 
> "git shortlog -n git-difftool* git-mergetool*" shows that my
> preferences and opinions are 44 times more important than yours in
> this area, whether you like it or not.  ;-p

That doesn't mean you are right.

This is an appeal to authority fallacy.

> Poking fun is my answer to such misguided seriousness.

I did not poke fun. I was just pointing out the fact that it wasn't an
argument.

> > > I don't have to imagine these use cases, they are very real.
> >
> > Show one.
> 
> Heh, I don't have to show you anything.

No you don't, but in a discussion with other people if you don't
substantiate your claims, then others have no rational reason to
consider them.

> Barring that, please use your imagination.  Imagine a custom file
> format for a graph-like data structure (both binary and text-like)
> that is able to use the full set of information for the purposes of
> resolving conflicts.  Private tools exist.  It's impossible to prove
> that they don't so I won't ask you or anyone else to do so.

So it's not a real use-case, it's an imaginary one.

Even if we do consider this imaginary use-case, you just swept the
problem under the carpet by saying "is able to use the full set of
information".

How?

Explain how this tool would use this information.

> > > This design choice is also in alignment with the general
> > > mergetool/difftool per-tool configuration paradigm.  If we didn't
> > > support per-tool, then it would be inconsistent.
> >
> > Can you do
> >
> >  1. mergetool.vimdiff3.keepBackup
> >  2. mergetool.vimdiff3.keepTemporaries
> >  3. mergetool.vimdiff3.writeToTemp
> >  4. mergetool.vimdiff3.prompt
> 
> can_merge()
> diff_cmd()
> merge_cmd()
> translate_merge_tool_path()
> list_tool_variants()
> exit_code_trustable()
> 
> mergetool.<tool>.cmd
> mergetool.<tool>.path
> mergetool.<tool>.trustExitCode

None of those are user preferences.

> We even have mergetool.meld.hasOutput which is completely tool-specific.

We don't have a user preference `mergetool.hasOutput`, or
`mergetool.vimdiff3.hasOutput` for that matter.
 
> I'm not asking, I'm telling you: the tool was designed around per-tool
> hooks.  This is per-tool behavior.  End of story.

And yet a tool cannot override `mergetool.keepBackup`.

Correct?

> > ?
> >
> > In fact the opposite is the case; not a *single* `mergetool.foo`
> > configuration can be done with `mergetool.<tool>.foo`.
> 
> That's not the right question.  This a behavior that can differ
> per-tool and thus this feature must reflect that.

Can it?

I am waiting for somebody to show *how*.

> There is a difference between top-level and per-tool behavior.

> This is a per-tool feature

No, that's your opinion. You haven't explained why.

> The way to think about it is that it's a per-tool feature

Ditto.

> We are not designing just for today; we must keep our eye on
> longer-term goals where the community tools can be improved.

Yes, but an actual future that might actually happen.

> Here is the "right" question to ask: Why shouldn't we have this
> flexibility?

Why shouldn't we have `mergetool.vimdiff3.keepBackup`?

> The heavy hammer of completely disabling the feature means that they
> will have crippled all of the other mergetools just because one of
> them couldn't cope,

Precisely *how* a tool couldn't cope?

> Are you willing to compromise on the small concession that we should
> allow per-tool overrides so that we can move forward and get this
> integrated?

We can move forward right now.

But no, I am 99.99% certain *nobody* will ever implement
should_automerge (), so I won't waste my time with it.

If you think otherwise, you spend your time implementing this on top of
my patch. That way it's clear who made the mistake.

> To be extremely clear -- is this discussion arguing between the
> following two strategies?
> 
> # Per-tool override + Global default
> 
> should_automerge () {
>     git config mergetool.$tool.automerge || git config
> mergetool.automerge || echo true
> }
> 
> # vs. Global default only
> 
> should_automerge () {
>     git config mergetool.automerge || echo true
> }
> 
> , or are we discussing a different aspect?  When spelled out in code,
> we're discussing whether or not we should have
> "mergetool.$tool.automerge ||" in there, and the argument for why not
> is pretty thin considering that use cases that would be harmed by not
> doing so exist.

I don't think mergetool.autoMerge should be any different from
mergetool.keepBackup; a single global user configuration suffices.

You say the argument against such per-tool configuration is thin, I say
the argument in favor is non-existent.

> Such a trivial difference is not a hill worth dying on so please route
> this energy into a patch so that we can get this integrated.

Nobody can tell me how I chose to spend my free time that I'm choosing to
contribute to this project altruistically.

> I suggest this compromise -- if adding "mergetool.$tool.automerge ||"
> to the logic is something you don't want to do then submit the patch
> without it and I'll submit a follow-up patch that adds the per-tool
> override.

That's not a compromise. That's a difference of opinion stated in the
code.

I don't have to convince you. And you don't have to convince me.

> I'd personally prefer to just have the patch include this from the
> get-go, though, so if we've managed to convince you then please take
> the shorter path to success.

And I would prefer that we don't waste time with hypothetical use-cases
that will never materialize.

We can add such configuration when the need actually arises.

But we can't always have what we prefer.

Cheers.

[1] https://en.wikipedia.org/wiki/Argument_from_authority

-- 
Felipe Contreras