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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html