shorten and colourise 'wip' output

RESOLVED FIXED

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
the current wip output is hard to scan due to it being multi-line and without colour.

ie. https://people-mozilla.org/~gszorc/hg-workflows/?full#hg-wip is nice.
Comment hidden (mozreview-request)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8812801 [details]
configwizard: shorten and colourise 'wip' output (bug 1319097);

https://reviewboard.mozilla.org/r/94398/#review94762

I've made the changes in this patch locally and can say that the new output looks great! I do have a few concerns about conflicts with upstream, however. And, this change breaks .t tests in several ways. Please run `./run-tests hgext/configwizard`.

While you're here, there's another option you may find useful (feel free to defer to a follow-up).

On Mercurial 3.9 or 4.0 (can't remember which), there is a new `sort()` revset function to control the sorted order of displayed revisions. For `hg wip`, the most interesting sorting order is `topo`, which sorts in reverse topological order (as opposed to revlog order). This means if you e.g. clone mozilla-unified then pull your user repo with all your draft changesets, all your draft changesets will show up further down the output corresponding with their relative order in the DAG. I find this extremely useful for `hg wip` and my local revset alias was wrapped with `sort(..., topo)`.

::: hgext/configwizard/__init__.py:717
(Diff revision 1)
> -            '{label("log.tag", if(tags," {tags}"))}'
> -            '{label("log.tag", if(fxheads," {fxheads}"))} '
> +        '{label("log.tags", if(tags," {tags}"))}'
> +        '{label("log.tags", if(fxheads," {fxheads}"))}'

Introducing a custom `log.tags` while the official is `log.tag` makes me a bit nervous. More on this in the next comment...

::: hgext/configwizard/__init__.py:728
(Diff revision 1)
> +    _set_color(cw, 'changeset.obsolete', 'none')
> +    _set_color(cw, 'changeset.public', 'blue')
> +    _set_color(cw, 'changeset.draft', 'green')
> +    _set_color(cw, 'log.branch', 'yellow')
> +    _set_color(cw, 'log.tags', 'yellow')
> +    _set_color(cw, 'log.bookmarks', 'yellow underline')
> +    _set_color(cw, 'wip.here', 'red')

I'm hesitant to define colors on built-in labels because I feel that is playing with fire. If upstream changes things, who knows what can happen.

I'd feel better if we prefixed all the names with `wip` so the customizations only apply to this alias.

FWIW, Mercurial 4.1 should be getting some form of `hg wip` as part of `hg display`. There's a good chance new formats/colors will be introduced for that. I'd hate for us to roll out conflicting names/colors. I think it's best to sit on a custom "namespace" for now. We can always unify with upstream later.
Attachment #8812801 - Flags: review?(gps) → review-
(Assignee)

Comment 3

2 years ago
(In reply to Gregory Szorc [:gps] from comment #2)
> On Mercurial 3.9 or 4.0 (can't remember which), there is a new `sort()`
> revset function to control the sorted order of displayed revisions.

it was added in 3.9.0.

updating the config file is trivial, but i can't figure out how to make the test output conditional (on hg39+) without duplicating large amounts of the tests and wrapping in feature blocks.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8812801 [details]
configwizard: shorten and colourise 'wip' output (bug 1319097);

https://reviewboard.mozilla.org/r/94398/#review94986

Thanks for incorporating the review comments!
Attachment #8812801 - Flags: review?(gps) → review+

Comment 7

2 years ago
mozreview-review
Comment on attachment 8813040 [details]
configwizard: shorten and colourise 'wip' output (bug 1319097);

https://reviewboard.mozilla.org/r/94558/#review94988

Hmmm. I'm guessing you meant to roll this into the other commit?

I'll do that locally then land for you. Please don't forget to close the review series.
Attachment #8813040 - Flags: review?(gps) → review+

Comment 8

2 years ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6ccb8a32a39b
configwizard: shorten and colourise 'wip' output ; r?gps
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.