Thanks for your reply! I've tried to address your comments point by point below:
> Nit: I'm not sure what this "merge driver" and "merge tool" mean, and the > term "merge driver" seems to have a different meaning in Mercurial. Can you > elaborate? I wasn't certain of the mercurial terminology, so I just kept what I had. Here "driver" is intended as something that runs unsupervised by default for merges, inserting markers for conflicts (i.e. the `merge` command). "Tool" is here meant as an application for resolving such conflicts (i.e. called by the `resolve` command), in this case a web GUI based one. If you can indicate what the terminology for mercurial is, I'll make the appropriate changes. The "driver" is here quite important for notebooks given that they are structured documents. A line-based text merge is likely to result in an invalid document, which is unfortunate since the internal JSON is not generally meant for human consumption. > These priorities seem too high compared to the other tools. I think these > tools should have quite low priority so they won't be selected as a general- > purpose merge tool. Most of the current tools registered have a priority in the range -10 to -1, but these are general purpose tools, not a file-format specific ones (as far as I can tell). What would be a reasonable priority for a file-format specific merge tool? I was not able to find any prior examples on this. > >> +# Default association of file types with merge tools >> + >> +[merge-patterns] >> + >> +**.ipynb = nbdime > > Registering merge-tools by default is good, but I don't think we should force > users to select them by default. Can you send new patch that only registers > merge-tools? Again, there seems to be no prior pattern to follow for file-format specific tools. Given that this is a file-format specific "driver", it should only be called for the given file extension. As such the `merge-patterns` entry seems mandatory for it to work as intended (calling `hg merge --tool=nbdime` will cause it to receive any file-types involved in the merge, not just notebooks, AFAICT). If the user would have to edit the configuration to enable the tool (i.e. to add the merge-patterns entry), there seems to be little meaning to adding the "driver" without this line (this defaults exist so that they can be used without editing the configuration). While I could send a new patch without the driver, I would say that having the "resolver" configured without the "driver" is an unlikely wanted setup for end users. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
