This is an archive of the discontinued Mercurial Phabricator instance.

docs: avoid `hg diff -r` in documentation about revsets
AbandonedPublic

Authored by martinvonz on Dec 9 2020, 10:09 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

I don't think hg diff -r a::b is a good use case for revsets because
it's not clear what it's supposed to do (as evidenced by the comment
we had there to explain what it meant).

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Dec 9 2020, 10:09 PM

I don't think hg diff -r a::b is a good use case for revsets because
it's not clear what it's supposed to do (as evidenced by the comment
we had there to explain what it meant).

I took the comment as giving an example of a command that takes two revisions will grab both ends, since the update parenthetical above it describes taking just one revision. I agree that's awfully subtle, though the "Specifying single revisions" section at the beginning calls this out and then says "see below". (Maybe a reference here back to that section to close the loop would be helpful. The text is so long, I tend to skip right to the examples and I'm guessing new users do too.)

I see the other diff related patches and like the direction, but should we replace this example with another that takes two revisions? (I don't have any suggestions about what.)

tests/test-extension.t
999

Unrelated, but this errors out for me. Anyone else having a problem? (It seems to work without -c, but that just prints the same revset help, so it's kind of a circular reference.)

martinvonz abandoned this revision.Dec 9 2020, 10:45 PM

I took the comment as giving an example of a command that takes two revisions will grab both ends, since the update parenthetical above it describes taking just one revision. I agree that's awfully subtle, though the "Specifying single revisions" section at the beginning calls this out and then says "see below". (Maybe a reference here back to that section to close the loop would be helpful. The text is so long, I tend to skip right to the examples and I'm guessing new users do too.)

Oh, I see, I think you're right that that's what they meant there. I don't think are any good examples of when revsets should resolve to two revisions. However, since we already have that behavior, we should document it, and the place we document it is here (as you pointed out). I'll just drop this patch.

martinvonz added inline comments.Dec 9 2020, 10:49 PM
tests/test-extension.t
999

It fails for me too. Ah! multirevs is an extension provided in this test case only :)