Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt rich, for handling terminal output #10461

Open
pradyunsg opened this issue Sep 11, 2021 · 11 comments
Open

Adopt rich, for handling terminal output #10461

pradyunsg opened this issue Sep 11, 2021 · 11 comments
Labels
C: output Related to what pip prints project: vendored dependency Related to a vendored dependency type: enhancement Improvements to functionality

Comments

@pradyunsg
Copy link
Member

Initial discussion at #10423

This is mostly an umbrella issue for tracking actually doing this, and for serving as a place for discussion whenever a downstream redistributor inevitably comes complaining about this. :)

The first step is obviously to vendor rich. Next, the most obvious change would be to use rich's output logging-related adapter, to replace as much of our custom output stuff with it as possible -- that'll allow using rich's nice markup in our output. At this point, we can close this issue.

The bulk of the benefits will come from further refactoring though:

  • replacing our spinners/progress bars with rich's spinners/progress bars.
  • replacing our user-input prompts with rich's prompts.
  • presenting tracebacks with rich's traceback highlighting.
  • utilising rich's status bar, to present proper status to the user.
@pradyunsg pradyunsg added type: enhancement Improvements to functionality project: vendored dependency Related to a vendored dependency C: output Related to what pip prints labels Sep 11, 2021
@pradyunsg
Copy link
Member Author

Oh, this is likely gonna bring back #8169; since rich only works on systems with proper threading support. And... the same will apply for pip. From #8169 (comment):

I do not think it is important to support non threading Pythons. We don't have to support it just because it is possible option.

@pfmoore
Copy link
Member

pfmoore commented Nov 6, 2021

+1 on just requiring threading to work, and not worrying beyond that. From #8169 (comment), threading is optional in Python 3.6, so we'd need to alter our support policy to either drop 3.6 slightly early, or to explicitly say that 3.6 without threading is not supported. But I don't think we need to do anything more than just say so and move on.

@uranusjr
Copy link
Member

uranusjr commented Nov 7, 2021

What if we make all Rich-related functionalities no-op on 3.6 (i.e. no progress bar etc.)? Honestly I don't feel rich displays aren't that essential anyway. Or would that be too tedious?

@pradyunsg
Copy link
Member Author

Well... 3.6 is end of life in 1 month and 2 weeks. I'm actually leaning toward dropping it entirely in 22.0 TBH.

And, I expect that we'd be using rich for a lot more than just progress bars (error message presentation is the primary motivator FWIW), so... I don't think I wanna do too much to make sure that certain configurations of Python 3.6 stay functional.

@pfmoore
Copy link
Member

pfmoore commented Nov 7, 2021

I agree, let's just drop 3.6 in 22.0 and be done with it.

@pradyunsg
Copy link
Member Author

Dropping 3.6 also means we can use dataclasses -- which... yes!

@pradyunsg
Copy link
Member Author

Okay, I spent a bit of time trying to port our spinners to use rich spinners, but... it's stupidly tricky, and I'm gonna kick the can down the road for that.

Dumping my WIP changes here (at least, the bits that still made sense) instead of my git stash because... it might be useful for someone else who might come along to pick this up
"""Spinners to present to the user that there is activity happening.
"""

import contextlib
import logging
import sys
import time
from typing import IO, Iterator

from pip._internal.utils.compat import WINDOWS
from pip._internal.utils.logging import get_indentation

logger = logging.getLogger(__name__)


# Used for dumb terminals, non-interactive installs (no tty), etc.
class NonInteractiveSpinner:
    def __init__(self, message: str) -> None:
        super().__init__()
        self._message = message

        # Keep track of time and presentation information.
        self._start_time = None
        self._last_tick = None
        self._finished = False

        # The gap in seconds between individual ticks (i.e. updates) to the spinner.
        # Ensure that we print updates occasionally, to act as a keep-alive for systems
        # that take lack-of-output as an indication that a task has frozen.
        self._tick_gap = 60.0

    def start(self) -> None:
        self.start_time = time.time()
        self.last_tick = self.start_time - self._tick_gap
        self._update("started")

    def _update(self, status: str) -> None:
        assert not self._finished
        now = time.time()
        if self._last_tick + self._tick_gap >= now:
            logger.info(
                "%s (%.1f seconds): %s",
                self._message,
                now - self._last_tick,
                status,
            )
            self.last_tick = now

    def finish(self, final_status: str) -> None:
        if self._finished:
            return
        self._update(f"finished with status '{final_status}'")
        self._finished = True


@contextlib.contextmanager
def open_spinner(message: str) -> Iterator[None]:
    should_use_interactive_spinner = (
        # The interactive spinner only makes sense if stdout is something that we'd
        # want to present an interactive spinner on.
        sys.stdout.isatty()
        # Since the interactive spinner is not piped through the logging system, do a
        # sanity check to ensure that we respect logging levels.
        and logger.getEffectiveLevel() <= logging.INFO
    )

    if should_use_interactive_spinner:
        spinner = InteractiveSpinner()
    else:
        spinner = NonInteractiveSpinner()

    try:
        spinner.start()
        yield
    except KeyboardInterrupt:
        spinner.stop("canceled")
        raise
    except Exception:
        spinner.stop("error")
        raise
    else:
        spinner.stop("done")


@contextlib.contextmanager
def hidden_cursor(file: IO[str]) -> Iterator[None]:
    # The Windows terminal does not support the hide/show cursor ANSI codes,
    # even via colorama. So don't even try.
    if WINDOWS:
        yield
    # We don't want to clutter the output with control characters if we're
    # writing to a file, or if the user is running with --quiet.
    # See https://github.com/pypa/pip/issues/3418
    elif not file.isatty() or logger.getEffectiveLevel() > logging.INFO:
        yield
    else:
        file.write(HIDE_CURSOR)
        try:
            yield
        finally:
            file.write(SHOW_CURSOR)

@pradyunsg
Copy link
Member Author

Another potential issue: tests are strongly coupled with the assumption that the spinner will spin on each line of output.

@H999

This comment was marked as off-topic.

@pradyunsg

This comment was marked as off-topic.

@H999

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: output Related to what pip prints project: vendored dependency Related to a vendored dependency type: enhancement Improvements to functionality
Projects
None yet
Development

No branches or pull requests

4 participants