Page MenuHomePhabricator

phabricator: add a "phabstatus" show view
Needs ReviewPublic

Authored by dlax on Fri, Nov 22, 11:00 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Summary

We add a "phabstatus" show view (called as "hg show phabstatus") which
renders a dag with underway revisions associated with a differential
revision and displays their status.

The revisions shown is a subset of that shown by "work" view, only
including revisions with known by Phabricator.

Diff Detail

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

Event Timeline

dlax created this revision.Fri, Nov 22, 11:00 AM
dlax updated this revision to Diff 18315.Fri, Nov 22, 11:02 AM

I like it.

Any idea why the changeset isn't colored, unlike hg show stack? It might also be a little more readable if the URI and status line were tabbed in, but maybe colored cset hashes would make that unnecessary?

I'm also interested in coloring the status value, but I can tinker with that after it's landed, unless you already have plans.

dlax planned changes to this revision.Fri, Nov 22, 4:09 PM

I like it.

Thanks!

Any idea why the changeset isn't colored, unlike hg show stack? It might also be a little more readable if the URI and status line were tabbed in, but maybe colored cset hashes would make that unnecessary?

Yes, I messed up with adding a custom template whereas I can actually reuse "hg show"'s one. Will fix (along with another issue in the algorithm I just found out.)

I'm also interested in coloring the status value, but I can tinker with that after it's landed, unless you already have plans.

You mean having a different color depending on status value, or a fixed one? I have no plan anyways, so if you have good ideas, I'll leave this up to you.

dlax updated this revision to Diff 18320.Fri, Nov 22, 4:13 PM
In D7506#110249, @dlax wrote:

I'm also interested in coloring the status value, but I can tinker with that after it's landed, unless you already have plans.

You mean having a different color depending on status value, or a fixed one? I have no plan anyways, so if you have good ideas, I'll leave this up to you.

Something like green for accepted/closed, red for needs revision, neutral-ish for needs review, strikethrough for abandoned.

I'm not sure why, but this version seems to also show obsolete revisions. I've got a bunch of x and * nodes in hg-committed right now. I didn't see that before, although that was on a different clone that I don't have access to now.

I'm not sure why, but this version seems to also show obsolete revisions. I've got a bunch of x and * nodes in hg-committed right now. I didn't see that before, although that was on a different clone that I don't have access to now.

After further review, the obsolete revisions have unstable children going way back, so that's why they show. But I'm super confused by the output. This isn't the whole output, but the middle or so that corresponds to the latest(!) commits:

: : : : | | : | | : : | o  d77be1 filemerge: fix a missing attribute usage
: : : : | | : | | : : | |  https://phab.mercurial-scm.org/D7465 Needs Review
: : : : | | : | | : : | | o  ef1696 makefile: use Python 3 by default (BC)
: : : : | | : | | : : | |/   https://phab.mercurial-scm.org/D7258 Accepted
+-+-+-------+-------+-+---o  640bae cleanup: update references to /help/ that should now be /helptext/
: : : : | | : | | : : | |    https://phab.mercurial-scm.org/D7472 Closed
: : : : | | : | | : : | | o  20a3bf rust-dirs: address failing tests for `dirs` impl with a temporary fix
: : : : | | : | | : : | | |  https://phab.mercurial-scm.org/D7503 Closed
: : : : | | : | | : : | | | o  e8373b (@) windows: further build fixes for the WiX installer
: : : : | | : | | : : | | | |  https://phab.mercurial-scm.org/D7509 Closed
: : : : | | : | | : : | | | | o  347b42 logcmdutil: call _exthook() in changesettemplater
: : : : | | : | | : : | | | | |  https://phab.mercurial-scm.org/D7505 Needs Review
: : : : | | : | | : : | | | | | o  4b1de5 phabricator: add a "phabstatus" show view
: : : : | | : | | : : | | | | | |  https://phab.mercurial-scm.org/D7506 Needs Review
: : : : | | : | | : : | | | | | | @  f6920a phabricator: add a "phabstatus" template keyword
: : : : | | : | | : : | | | | | | |  https://phab.mercurial-scm.org/D7507 Needs Review
+-+-+-------o---+---+ | | | | | | |  ce20b8 format: format commands.py, which recently regressed
: : : : | |  / / / / / / / / / / /   https://phab.mercurial-scm.org/D7064 Closed

Show work looks like (truncated to latest):

$ hg show work
@  f6920a phabricator: add a "phabstatus" template keyword
o  4b1de5 phabricator: add a "phabstatus" show view
o  347b42 logcmdutil: call _exthook() in changesettemplater
o  e8373b (@) windows: further build fixes for the WiX installer
o  20a3bf rust-dirs: address failing tests for `dirs` impl with a temporary fix
o  640bae cleanup: update references to /help/ that should now be /helptext/
o    7eb701 merge with stable
|\
| o    88a306 (stable) singlehead: making config item a bool again
| :\
| : \
| : :\
| : : : o  06aac2 xxx-pytype: annotate various things flagged for future examination
+-------o  6c9c22 webutil: drop an extra parameter on `join()`
| : : : o  ef1696 makefile: use Python 3 by default (BC)
| : : : | o  d77be1 filemerge: fix a missing attribute usage
| : : : |/
+-------o  538726 filemerge: drop a default argument to appease pytype

And show stack looks like:

$ hg show stack
  @  f6920 phabricator: add a "phabstatus" template keyword
  o  4b1de phabricator: add a "phabstatus" show view
  o  347b4 logcmdutil: call _exthook() in changesettemplater
  o  e8373 (@) windows: further build fixes for the WiX installer
  o  20a3b rust-dirs: address failing tests for `dirs` impl with a temporary fix
  o  640ba cleanup: update references to /help/ that should now be /helptext/
 /   (stack base)
o  7eb70 merge with stable

Is the goal to limit to the current stack, or all non public commits? I can see a use case for both.

dlax planned changes to this revision.Sat, Nov 23, 5:15 AM

I'm not sure why, but this version seems to also show obsolete revisions. I've got a bunch of x and * nodes in hg-committed right now. I didn't see that before, although that was on a different clone that I don't have access to now.

After further review, the obsolete revisions have unstable children going way back, so that's why they show. But I'm super confused by the output. This isn't the whole output, but the middle or so that corresponds to the latest(!) commits:

Hm, I don't have this issue. Even with visible obsolete revisions with a phab differential, the output seem consistent with "show work".
The revisions shown by the "phabstatus" view is just a subset of that of "work" view (subset of revisions with a differential). Do you see more revisions in "phabstatus" than in "work" or am I misunderstanding something?
Or perhaps this is because of a wrong sorting of the filtered set? I'll make sure the final is still topo sorted and send another version in a moment.

Is the goal to limit to the current stack, or all non public commits? I can see a use case for both.

I don't use the "stack" view often, so my goal is to map to the "work" view which I use more. But perhaps it'd make sense to have "hg show phabwork" and "hg show phabstack"? (We can rename the view, and add the other one later on.)

dlax edited the summary of this revision. (Show Details)Sat, Nov 23, 5:26 AM
dlax updated this revision to Diff 18338.
In D7506#110351, @dlax wrote:

The revisions shown by the "phabstatus" view is just a subset of that of "work" view (subset of revisions with a differential). Do you see more revisions in "phabstatus" than in "work" or am I misunderstanding something?

No, there are 89 entries in work, and 35 in phabstatus. (I used ../hg show phabstatus | egrep '[0-9a-f]{6}' | wc -l to tame the output.) My confusion was that things that are direct descendants were showing like they were all separate heads. (e.g. the revision marked @ and the @ bookmark 3 above it in phabstatus should be a direct line.) This latest update made things look way more sane. The remaining old junk in my output is likely because I tend to leave anonymous branches laying around, and it's adding upstream parents for context.

Or perhaps this is because of a wrong sorting of the filtered set? I'll make sure the final is still topo sorted and send another version in a moment.

Is the goal to limit to the current stack, or all non public commits? I can see a use case for both.

I don't use the "stack" view often, so my goal is to map to the "work" view which I use more. But perhaps it'd make sense to have "hg show phabwork" and "hg show phabstack"? (We can rename the view, and add the other one later on.)

I had forgotten about the extension TBH. Because I tend to leave junk around, I'd probably use the stack view much more. Narrowing to the stack also seems useful now that the author has been removed since the first revision of this. So I think having both is a good idea.

pulkit accepted this revision.Tue, Dec 10, 10:46 AM
This revision is now accepted and ready to land.Tue, Dec 10, 10:46 AM
pulkit requested changes to this revision.Tue, Dec 10, 10:57 AM

test-check-module-imports.t says hi!

This revision now requires changes to proceed.Tue, Dec 10, 10:57 AM
dlax updated this revision to Diff 18589.Tue, Dec 10, 2:46 PM