Closed Bug 1422302 Opened 7 years ago Closed 7 years ago

Create a new 'mozterm' module for shared blessings-based decorations

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(2 files)

Bug 1422302 - Create python/mozterm for sharing terminal blessings across modules
59 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1422302 - Move mozbuild.controller.building.Footer to mozterm
59 bytes, text/x-review-board-request
gps
: review+
Details 
I'd like to re-use things like the build footer in some of the mozlog formatters. It would be nice if there were a module of common blessings based widgets that could be shared across modules and mach commands.

This could also be a place to put mozlint's NullTerminal() which is a drop-in replacement for blessings.Terminal() that does no formatting. I'd like to implement something like:

    from mozterm import Terminal
    t = Terminal()
    print(t.red("foo"))        # no colors if not supported
    t = Terminal(raises=True)  # raises if not supported
Attachment #8933713 - Flags: review?(core-build-config-reviews)
Attachment #8933714 - Flags: review?(core-build-config-reviews)
Comment on attachment 8933713 [details]
Bug 1422302 - Create python/mozterm for sharing terminal blessings across modules

I'll look at this (since I don't want to force anyone else to look at terminal code and I kinda have it paged in right now).
Attachment #8933713 - Flags: review?(core-build-config-reviews) → review?(gps)
Attachment #8933714 - Flags: review?(core-build-config-reviews) → review?(gps)
Comment on attachment 8933713 [details]
Bug 1422302 - Create python/mozterm for sharing terminal blessings across modules

https://reviewboard.mozilla.org/r/204656/#review211124

::: python/mozterm/mozterm/terminal.py:45
(Diff revision 3)
> +    if disable_styling:
> +        return NullTerminal(**kwargs)
> +
> +    try:
> +        import blessings
> +    except:

Nit: this should be `except Exception`. Bare word `except:` catches `KeyboardInterrupt` and `SystemExit`, which is almost never wanted. I'm kinda surprised a linter isn't catching that...
Attachment #8933713 - Flags: review?(gps) → review+
Comment on attachment 8933714 [details]
Bug 1422302 - Move mozbuild.controller.building.Footer to mozterm

https://reviewboard.mozilla.org/r/204658/#review211126

::: python/mozterm/test/test_widgets.py:7
(Diff revision 3)
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +from __future__ import absolute_import, unicode_literals
> +
> +from StringIO import StringIO

We would ideally be using `io.StringIO` or `io.BytesIO` for Python 3 compatibility. But let's worry about this another day.

::: python/mozterm/test/test_widgets.py:33
(Diff revision 3)
> +    footer.write([
> +        ('dim', 'foo'),
> +        ('green', 'bar'),
> +    ])
> +    value = terminal.stream.getvalue()
> +    expected = "\x1b7\x1b[2mfoo\x1b(B\x1b[m \x1b[32mbar\x1b(B\x1b[m\x1b8"

I /think/ this will depend on the terminfo database being available and having an entry for `xterm-256color`. Otherwise blessings won't know which byte sequences to emit.

But having a terminfo database is pretty common and `xterm-256color` is pretty ubiquitous. So this should be fine. I figured I'd leave a comment so you know of the issue. It might be worth adding an inline comment. Or even better, adding code to skip the test of we can't find info in the terminfo database.
Attachment #8933714 - Flags: review?(gps) → review+
Comment on attachment 8933714 [details]
Bug 1422302 - Move mozbuild.controller.building.Footer to mozterm

https://reviewboard.mozilla.org/r/204658/#review211126

> We would ideally be using `io.StringIO` or `io.BytesIO` for Python 3 compatibility. But let's worry about this another day.

Might as well fix it while I'm uploading a new revision anyway. This also reminded me that I forgot to enable the flake8 linter. We need to figure out how to fix bug 1367092 :/

> I /think/ this will depend on the terminfo database being available and having an entry for `xterm-256color`. Otherwise blessings won't know which byte sequences to emit.
> 
> But having a terminfo database is pretty common and `xterm-256color` is pretty ubiquitous. So this should be fine. I figured I'd leave a comment so you know of the issue. It might be worth adding an inline comment. Or even better, adding code to skip the test of we can't find info in the terminfo database.

Looks like blessings will raise `curses.error` if the entry isn't found. I'll catch it and skip.
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ea36374ea21a
Create python/mozterm for sharing terminal blessings across modules r=gps
https://hg.mozilla.org/integration/autoland/rev/7d01faca3c8e
Move mozbuild.controller.building.Footer to mozterm r=gps
https://hg.mozilla.org/mozilla-central/rev/ea36374ea21a
https://hg.mozilla.org/mozilla-central/rev/7d01faca3c8e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: