facebook / sapling

A Scalable, User-Friendly Source Control System.
https://sapling-scm.com
GNU General Public License v2.0
5.91k stars 270 forks source link

Documentation and defaults for merge tools could be improved #852

Open adamh-oai opened 3 months ago

adamh-oai commented 3 months ago

On a Mac with XCode installed, Sapling will use filemergexcode as the default interactive merge tool. We do not love this tool, but struggled to configure use of standard conflict markers. After looking through the code we eventually found this works:

[ui]
merge = internal:merge3
merge:interactive = internal:merge3

The separate config for interactive vs non-interactive merges does not seem to be mentioned in either the sapling or mercurial docs.

The defaults seem to come from this built-in config.

My 2c is that having the setup for these tools (arguments etc) available by default is great, but enabling them by default is not desirable, especially given how opaque the config is. I'd prefer to add a ui.merge = filemergexcode line to make it explicit.

quark-zju commented 3 months ago

Thanks for bringing this up. We do plan to make it more friendly to configure. For example, the first time a merge tool is being used and if you're in an interactive terminal, maybe we should ask you what tool to use.

Regarding on the default, I do think an explicitly configured tool is a better choice. There are some documents (like https://www.araxis.com/merge/macos/integrating-with-other-applications.en#Mercurial) that rely on the implicit tool selection, and some of our internal users expect that behavior (they complained when we accidentally broke the implicit tool selection). So making it explicit would be a breaking change. Perhaps we should at least print a hint about why a tool is changed and how to update the config.

adamh-oai commented 3 months ago

Makes sense. I'm less concerned about the choice of default, than about how it was difficult to figure out why it was selected / how to change it.