On Sat, 12 Jan 2019 09:26:25 -0800, Ludovic Chabant wrote:
> Thanks for the comments!
> 
> > Maybe need to check the .gui flag of the tool. Otherwise the console
> > would be messed up if the tool wasn't a GUI program.
> 
> Good point, although the existing extdiff code doesn't seem to be checking 
> that, no? So should I do that in a separate patch?

A separate patch seems fine. To make clear, there's no need to check if the
tool is console-based or not unless multiple diff processes are spawned
simultaneously. Currently the .gui flag is tested only by filemerge.py.

> > Well, I think it will produce 8 modes,
> > dir-or-file x revpair-or-revrage x prompt-or-not.
> > 
> > And we can even get rid of the dir-or-file mode from command flags by
> > registering two tools, one for dir-diff and the other for per-file-diff.
> > I don't know if that is a better idea, but we'll never use the per-file
> > mode if the tool had a decent support for directory diff. So it's a tool-
> > specific property at some level.
> 
> It's not super important, but I disagree about the second-to-last-point here, 
> since I actually wrote this patch with the intent of using per-file diffs 
> with tools like BeyondCompare and vim+DirDiff, which both handle dir diffs 
> pretty well. It's just that 95% of the time I want to see the diff of _all_ 
> the files in a revision, and going through a dir-diff UI just adds 
> unnecessary actions/clicks to get all those diffs to open.
> The only times I want a dir-diff where I'm going to only look at _some_ of 
> the files is when diffing a big merge.
> 
> As a result, I was indeed planning on configuring per-file-diffing by default 
> in my .hgrc for those tools, while occasionally passing a dir-diff argument 
> when appropriate. Sure, I could make 2 different tools in my .hgrc if we 
> didn't want to expose new options to the CLI, but for some reason it feels 
> wrong to force users to edit their .hgrc for that.
> 
> I see your point about modes vs flags. Like I said, I don't have very strong 
> opinions about it so I guess I'll change the second patch and use some flags 
> instead. Are we going for --per-file and --confirm then?

To all, any thoughts?

I feel uncomfortable these days as there's little response to this kind of
UI patches. I'm not a good UI designer.

> Note that the tool config in .hgrc would look a bit less elegant as a result, 
> like this:
> 
> cmd.blah=/path/to/blah
> opts.blah=--some-option -z
> per-file.blah=true
> confirm.blah=true
> 
> So instead I'm proposing a new config that specifies options for the hg 
> extdiff command itself, like this:
> 
> cmd.blah=/path/to/blah
> opts.blah=--some-option -z
> hgopts.blah=--per-file --confirm
> 
> ...but I don't know if that opens up potential abuse or something.  What do 
> you think?

The latter looks worse for me. Instead, you can use [alias] to pass in
arguments to hg commands.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to