Danek Duvall wrote: > I'm implementing a -r option to webrev, which, generally, takes a revision > range and creates the webrev from that range, without going off to the > parent to figure out what's changed. It would thus also allow you to get > the webrev of an existing changeset which may or may not be common to both > your workspace and its parent.
Is this perhaps confusing because you're allowing the basis of comparison (ie the left hand side) to be implicitly derived, when it should be explicitly specified? Currently, the default behavior of webrev is to choose the "parenttip," which is generally going to be the tipmost node on the same branch as the current head which is not also outgoing. Similarly, the default right hand side for comparison is the working directory. If you allow either or both to be overridden by explicit args, don't you eliminate the ambiguity described below? The hg bundle command uses the "--base" argument (with no short form) to allow something similar on the lhs, and -r for the rhs, but does not deal with the working dir at all. This defers discussion of -r. Like Bill, I think that webrev should behave as hg diff. Because, after all, what is webrev, other than a way to intuitively display diffs? And even nonintuitively, I suppose; we provide several different display formats, and at least one of them [patch] SHOULD match "hg diff" output verbatim, for the same -r spec. For hg diff, you may use two different -r args, where "-r lhs -r rhs" is an alternative way to say "-r lhs:rhs." So to circle back around, and ultimately agree with Bill and the hg diffs crowd: if you allow for use of the parentrevspec extension, then you get: arg lhs rhs default parenttip working dir -r a a working dir -r a:b a b ...where the only thing "missing" is "parenttip:b." And you can, essentially, get that by using "a:b," where a is set to the parentrevspec on your first outgoing node. Other than orthogonality, is there a compelling reason to make that case any easier to specify? If so, you could certainly add -r :b parenttip b to the above table, but this WOULD conflict with hg diff usage, wherein ":x" means "0:x." So maybe you introduce "ptip" as a not-really-a-tag special case? For the obviously-useful single-mq-patch example, you could define a "-p" arg (meaning "patch"), such that -p patch would be equivalent to -r patch^:patch ? --Mark > Anyway, after some discussion with a few folks, there's a bit of > disagreement about exactly what the arguments to -r mean. As I currently > have it implemented, > > -r r1 changes from p(r1) to r1 > -r r1: changes from p(r1) to tip > -r r1:r2 changes from p(r1) to r2 > > where p(r) means "first parent of r". This is analogous to how the -r > option to "hg log" works. In particular, "-r r1" would give you the webrev > of the changeset r1, which is a useful concept for folks using MQ (where > changesets are named via tags), or if you want to create a webrev of a > historical changeset. However, there's no way to get a webrev including > any changes made to the working directory, only changes already committed > to one or more consecutive changesets. > > However, some folks suggested that "hg diff" may be a better template: > > -r r1 changes from r1 to WD > -r r1: changes from r1 to tip > -r r1:r2 changes from r1 to r2 > > The disadvantage (as I see it) is that you'd have to figure out by hand > what the parent of any given changeset is, if you want the short form for > MQ or historical usage (though the parentrevspec extension is useful here > -- "-r r1" in the first implementation could be written as "-r r1^:r1" in > the second. > > A compromise could be something like this: > > -r r1 changes from p(r1) to r1 > -r r1: changes from p(r1) to WD > -r r1:r2 changes from p(r1) to r2 > > That is, like the current implementation, except that the unbounded range > would include the changes to the working directory. You could limit it to > tip by using "-r r1:tip", as the "tip" tag is always defined. The > disadvantage is that nothing else in mercurial really acts like this, so it > might confuse everyone. > > And none of these try to define "appropriate" behavior for branching > repositories, like those under pbranch control (mostly because I don't have > any experience with pbranch yet, and cadmium can't deal with branches at > all, to my knowledge). My feeling after spending a few minutes with > pbranch is that you can either use -r to specify the range of revisions of > the branch (or some subset thereof), or I can add a -b option later on to > simplify generating the webrev for the entirety of the branch. > > Does anyone have any feeling for this? I'm tempted to implement the third > option, and if people can't seem to figure it out, change it to the second, > "diff-style" option, and see if it's any better. And that's probably what > I'll do if I don't get a whole lot of response, so if you care, please > speak up. _______________________________________________ tools-discuss mailing list tools-discuss@opensolaris.org