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

Improve presentation of diagnostic errors using rich #10703

Merged
merged 10 commits into from
Dec 14, 2021
1 change: 1 addition & 0 deletions news/10703.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Start using Rich for presenting error messages in a consistent format.
10 changes: 5 additions & 5 deletions src/pip/_internal/cli/base_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,16 @@ def exc_logging_wrapper(*args: Any) -> int:
status = run_func(*args)
assert isinstance(status, int)
return status
except PreviousBuildDirError as exc:
logger.critical(str(exc))
except DiagnosticPipError as exc:
logger.error("[present-diagnostic]", exc)
logger.debug("Exception information:", exc_info=True)

return PREVIOUS_BUILD_DIR_ERROR
except DiagnosticPipError as exc:
return ERROR
except PreviousBuildDirError as exc:
logger.critical(str(exc))
logger.debug("Exception information:", exc_info=True)

return ERROR
return PREVIOUS_BUILD_DIR_ERROR
except (
InstallationError,
UninstallationError,
Expand Down
155 changes: 109 additions & 46 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
import configparser
import re
from itertools import chain, groupby, repeat
from typing import TYPE_CHECKING, Dict, Iterator, List, Optional
from typing import TYPE_CHECKING, Dict, List, Optional, Union
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved

from pip._vendor.requests.models import Request, Response
from pip._vendor.rich.console import Console, ConsoleOptions, RenderResult
from pip._vendor.rich.markup import escape
from pip._vendor.rich.text import Text

if TYPE_CHECKING:
from hashlib import _Hash
Expand All @@ -22,75 +25,140 @@ def _is_kebab_case(s: str) -> bool:
return re.match(r"^[a-z]+(-[a-z]+)*$", s) is not None


def _prefix_with_indent(prefix: str, s: str, indent: Optional[str] = None) -> str:
if indent is None:
indent = " " * len(prefix)
def _prefix_with_indent(
s: Union[Text, str],
console: Console,
*,
prefix: str,
indent: str,
) -> Text:
if isinstance(s, Text):
text = s
else:
assert len(indent) == len(prefix)
message = s.replace("\n", "\n" + indent)
return f"{prefix}{message}\n"
text = console.render_str(s)

return console.render_str(prefix, overflow="ignore") + console.render_str(
f"\n{indent}", overflow="ignore"
).join(text.split(allow_blank=True))


class PipError(Exception):
"""The base pip error."""


class DiagnosticPipError(PipError):
"""A pip error, that presents diagnostic information to the user.
"""An error, that presents diagnostic information to the user.

This contains a bunch of logic, to enable pretty presentation of our error
messages. Each error gets a unique reference. Each error can also include
additional context, a hint and/or a note -- which are presented with the
main error message in a consistent style.

This is adapted from the error output styling in `sphinx-theme-builder`.
"""

reference: str

def __init__(
self,
*,
message: str,
context: Optional[str],
hint_stmt: Optional[str],
attention_stmt: Optional[str] = None,
reference: Optional[str] = None,
kind: 'Literal["error", "warning"]' = "error",
reference: Optional[str] = None,
message: Union[str, Text],
context: Optional[Union[str, Text]],
hint_stmt: Optional[Union[str, Text]],
note_stmt: Optional[Union[str, Text]] = None,
link: Optional[str] = None,
) -> None:

# Ensure a proper reference is provided.
if reference is None:
assert hasattr(self, "reference"), "error reference not provided!"
reference = self.reference
assert _is_kebab_case(reference), "error reference must be kebab-case!"

super().__init__(f"{reference}: {message}")

self.kind = kind
self.reference = reference

self.message = message
self.context = context

self.reference = reference
self.attention_stmt = attention_stmt
self.note_stmt = note_stmt
self.hint_stmt = hint_stmt

def __str__(self) -> str:
return "".join(self._string_parts())

def _string_parts(self) -> Iterator[str]:
# Present the main message, with relevant context indented.
yield f"{self.message}\n"
if self.context is not None:
yield f"\n{self.context}\n"
self.link = link
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hope is that, in the future, these can be generated using the reference and somehow be synchronised with the documentation... but let's keep this as is for now. :)


# Space out the note/hint messages.
if self.attention_stmt is not None or self.hint_stmt is not None:
yield "\n"
super().__init__(f"<{self.__class__.__name__}: {self.reference}>")

if self.attention_stmt is not None:
yield _prefix_with_indent("Note: ", self.attention_stmt)
def __repr__(self) -> str:
return (
f"<{self.__class__.__name__}("
f"reference={self.reference!r}, "
f"message={self.message!r}, "
f"context={self.context!r}, "
f"note_stmt={self.note_stmt!r}, "
f"hint_stmt={self.hint_stmt!r}"
")>"
)

def __rich_console__(
self,
console: Console,
options: ConsoleOptions,
) -> RenderResult:
colour = "red" if self.kind == "error" else "yellow"

yield f"[{colour} bold]{self.kind}[/]: [bold]{self.reference}[/]"
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
yield ""

if not options.ascii_only:
# Present the main message, with relevant context indented.
if self.context is not None:
yield _prefix_with_indent(
self.message,
console,
prefix=f"[{colour}]×[/] ",
indent=f"[{colour}]│[/] ",
)
yield _prefix_with_indent(
self.context,
console,
prefix=f"[{colour}]╰─>[/] ",
indent=f"[{colour}] [/] ",
)
else:
yield _prefix_with_indent(
self.message,
console,
prefix="[red]×[/] ",
indent=" ",
)
else:
yield self.message
if self.context is not None:
yield ""
yield self.context

if self.note_stmt is not None or self.hint_stmt is not None:
yield ""

if self.note_stmt is not None:
yield _prefix_with_indent(
self.note_stmt,
console,
prefix="[magenta bold]note[/]: ",
indent=" ",
)
if self.hint_stmt is not None:
yield _prefix_with_indent("Hint: ", self.hint_stmt)
yield _prefix_with_indent(
self.hint_stmt,
console,
prefix="[cyan bold]hint[/]: ",
indent=" ",
)

if self.link is not None:
yield ""
yield f"Link: {self.link}"


#
Expand All @@ -115,15 +183,13 @@ class MissingPyProjectBuildRequires(DiagnosticPipError):

def __init__(self, *, package: str) -> None:
super().__init__(
message=f"Can not process {package}",
context=(
message=f"Can not process {escape(package)}",
context=Text(
"This package has an invalid pyproject.toml file.\n"
"The [build-system] table is missing the mandatory `requires` key."
),
attention_stmt=(
"This is an issue with the package mentioned above, not pip."
),
hint_stmt="See PEP 518 for the detailed specification.",
note_stmt="This is an issue with the package mentioned above, not pip.",
hint_stmt=Text("See PEP 518 for the detailed specification."),
)


Expand All @@ -134,16 +200,13 @@ class InvalidPyProjectBuildRequires(DiagnosticPipError):

def __init__(self, *, package: str, reason: str) -> None:
super().__init__(
message=f"Can not process {package}",
context=(
message=f"Can not process {escape(package)}",
context=Text(
"This package has an invalid `build-system.requires` key in "
"pyproject.toml.\n"
f"{reason}"
),
hint_stmt="See PEP 518 for the detailed specification.",
attention_stmt=(
"This is an issue with the package mentioned above, not pip."
f"pyproject.toml.\n{reason}"
),
note_stmt="This is an issue with the package mentioned above, not pip.",
hint_stmt=Text("See PEP 518 for the detailed specification."),
)


Expand Down
Loading