1319097 - shorten and colourise 'wip' output

Status: RESOLVED FIXED

(Developer Services :: Mercurial: configwizard, defect)

Description

[:glob ✱]

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 1

[:glob ✱]

Attachment: configwizard: shorten and colourise 'wip' output (bug 1319097);

In order to improve readability of the 'wip' command, switch from multi-line to
single-line output, and use colours to make parts of the output distinguishable.

Review commit: https://reviewboard.mozilla.org/r/94398/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94398/

Comment 2

[Gregory Szorc [:gps]]

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.

Comment 3

[:glob ✱]

(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 4

[:glob ✱]

Attachment: configwizard: shorten and colourise 'wip' output (bug 1319097);

Fix test-wip.t

Review commit: https://reviewboard.mozilla.org/r/94558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94558/

Comment 5

[:glob ✱]

Comment on attachment 8812801 [details]
configwizard: shorten and colourise 'wip' output (bug 1319097);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94398/diff/1-2/

Comment 6

[Gregory Szorc [:gps]]

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!

Comment 7

[Gregory Szorc [:gps]]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6ccb8a32a39b
configwizard: shorten and colourise 'wip' output ; r?gps