Charles Bailey wrote:
> On Tue, Apr 22, 2014 at 01:53:46AM -0500, Felipe Contreras wrote:
> > Charles Bailey wrote:
> > > On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote:
> > > >
> > > > This is what I get when a tool is not working:
> > > >
> > > > Documentation/config.txt seems unchanged.
> > > > Was the merge successful? [y/n]
> > >
> > > Does this happen now even with merge tools for which we do trust the
> > > exit code? If so, my original concern is addressed.
> >
> > Which tools are those?
>
> I didn't remember off hand, but checking the mergetools directory
> suggests that kdiff3 is one (there's no call to check_unchanged). I
> stopped checking after I found one.
So:
% cat > ~/bin/kdiff3 <<-\EOF
#!/bin/sh
false
EOF
% chmod +x ~/bin/kdiff3
% git -c merge.tool=kdiff3 mergetool
merge of Documentation/config.txt failed
Continue merging other unresolved paths (y/n) ?
> > You don't see anything wrong with asking the user *every single time* he
> > runs
> > `git mergetool`, even though he *already told us* which tool to use?
> >
> > If so, I'm pretty sure everybody else disagrees with you.
>
> I think that you may have misunderstood me. As I said, I've no
> particular objections to changing the default (subject a few concerns
> that may or may not still apply).
>
> Having said that, the fact that the user has configured the merge tool
> doesn't mean that he necessarily doesn't want to see the prompt.
Not necessarily, but in 99% of the cases it does. And for the remaining there's
always mergetool.prompt = true.
> In a part of my reply which you snipped, I said that it's sometimes the
> particular file that's due to be resolved that might prompt a user to want to
> skip launching the tool.
That's a possibility, however, in almost all the situations I've wanted to stop
a merge, it's *after* I've seen the actual conflicts in the file.
Anyway, I've revisited the code, and it's only now that I've realized that this:
Hit return to start merge resolution tool (kdiff3):
Is not actually asking me for the tool I want to use; the value in parenthesis
is not the default, and I can't type in another tool.
So the purpose of the prompt is very different from what I was thinking, yet I
still think the value of such prompt is marginal at best.
> Either way, I think we shouldn't unconditionally override an explicitly
> set mergetool.prompt and if we are (effectively) changing the default we
> should probably update the documentation to say so as well.
An explicitly set mergetool.prompt = true would override the default. See the
patch.
I looked, the documentation doesn't mention any default. We could add it, but I
don't think it's necesarily part of this patch.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
