> Sorry for late review. Almost all devs were inactive while holidays. No worries! Between the holidays and the 4.8.2 release, I figured unsolicited new features were a low priority.
> First, can you break this into a series of a few patches? > > https://www.mercurial-scm.org/wiki/ContributingChanges#Organizing_patches > > I expect the first patch will probably reorder or extract the current behavior > to function such that new modes can be added. Sure! I wasn't sure exactly what qualifies as multi vs single patch changes. > I don't think "--mode <char>" is good ui. I don't have nice idea, but maybe > it can be split into two flags (e.g. --per-file and --confirm/--interactive)? > > And the config option can specify if the tool support directory diff, for > example. If the tool doesn't support directory diff, and if it's gui, spawn > processes per file by default. I was actually debating with myself between the 2 ways of specifying the diff mode, and eventually I settled on the --mode option. Here's the rationale (although I can be easily convinced to use the alternative): - A --mode option is easier to later extend to more ways to diff things, like for example a mode where, when you pass a revision range, it will diff each revision one by one (invoking the external program N times if there are N revisions in the range). Different flags that may or may not be compatible between each other are more confusing IMHO. - The --confirm option doesn't do anything if you don't also pass --per-file... I don't know if there are other aspects of the hg cli that have this? Now I realize that I also fell short of that downside since "--mode p" doesn't do anything either, but the point is that if we want something to represent "per file" and "per file with prompt", 2 modes do that better than 2 flags (and I could change my patch to have "p" be a mode of its own instead of modifying the "f" mode). But like I was saying, I'm open to changing it. I figured this would probably be the main debate with this patch. > Unrelated changes? Woops, yes. Sorry, I didn't correctly check my cmdline arguments when sending the patch. -- l u d o . . 8 0 17 80 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
