Skip to content

Commit

Permalink
fix: correctly print multi-line errors (#270)
Browse files Browse the repository at this point in the history
* fix: correctly print multi-line errors

The bug manifested in a situation like this:

- A long progress() message is printed, say "AAAAAAAA";
- An error is emitter, and the result of str(error) is a multi-line string
  where the first line is shorter than the previously-emitted progress()
  message. Example: "BBBB\nCCCC";
- The newline in the error message is not correctly handled by the printer,
  meaning that after "BBBB" is written to the terminal the remainder of the
  line is not cleared. So the end result looks like:
  > BBBBAAAA
  > CCCC

The "proper" fix for this would mean a big rework on the printer to properly
support multi-line messages; for now, fix only the error case by "manually"
splitting the error message and printing each line individually.

Fixes #263
---------

Co-authored-by: Matt Culler <[email protected]>
  • Loading branch information
tigarmo and mattculler authored Sep 5, 2024
1 parent 385785d commit 0ca6940
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 3 deletions.
8 changes: 5 additions & 3 deletions craft_cli/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ def ended_ok(self) -> None:
"""Finish the messaging system gracefully."""
self._stop()

def _report_error(self, error: errors.CraftError) -> None:
def _report_error(self, error: errors.CraftError) -> None: # noqa: PLR0912 (too many branches)
"""Report the different message lines from a CraftError."""
if self._mode in (EmitterMode.QUIET, EmitterMode.BRIEF, EmitterMode.VERBOSE):
use_timestamp = False
Expand All @@ -719,8 +719,10 @@ def _report_error(self, error: errors.CraftError) -> None:
use_timestamp = True
full_stream = sys.stderr

# the initial message
self._printer.show(sys.stderr, str(error), use_timestamp=use_timestamp, end_line=True)
# The initial message. Print every line individually to correctly clear
# previous lines, if necessary.
for line in str(error).splitlines():
self._printer.show(sys.stderr, line, use_timestamp=use_timestamp, end_line=True)

if isinstance(error, errors.CraftCommandError):
stderr = error.stderr
Expand Down
9 changes: 9 additions & 0 deletions examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,15 @@ def example_30():
time.sleep(0.001)


def example_31():
"""Multiline error message."""
emit.progress("Setting up computer for build...")
time.sleep(1)
emit.progress("A long progress message")
time.sleep(6)
raise CraftError("Error 1\nError 2")


# -- end of test cases

if len(sys.argv) < 2:
Expand Down
17 changes: 17 additions & 0 deletions tests/integration/test_messages_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,23 @@ def test_error_unexpected_debugish(capsys, mode):
assert_outputs(capsys, emit, expected_err=expected, expected_log=expected)


@pytest.mark.parametrize("output_is_terminal", [True])
def test_error_multiline_brief(capsys):
emit = Emitter()
emit.init(EmitterMode.BRIEF, "testapp", GREETING)
emit.progress("A very long message detailing the current task.")
error = CraftError("Error line 1.\nError line 2.", logpath_report=False)
emit.error(error)

expected = [
Line("A very long message detailing the current task.", permanent=False),
# The error message is split on two separate lines.
Line("Error line 1.", permanent=True),
Line("Error line 2.", permanent=True),
]
assert_outputs(capsys, emit, expected_err=expected, expected_log=expected)


@pytest.mark.parametrize(
"mode",
[
Expand Down

0 comments on commit 0ca6940

Please sign in to comment.