Skip to content

Commit

Permalink
messages: replace tabs with spaces in open_stream() (#151)
Browse files Browse the repository at this point in the history
The tabs complicate the internal book-keeping because "\t12" has 3
characters but is printed to the terminal with a varying number of
spaces (depending on the user's tabwidth). For the pipe reader used
in open_stream(), this issue manifested as a bug where lines containing
tabs would result in multiple lines being printed (instead of just the
last one), which breaks the "ephemeral" aspect of the stream.

Note that this discovery implies that we should probably revisit this
tab issue in more places; in particular, the Printer does multiple
comparisons between the len() of the text and the terminal width, so
if the text has tabs that comparison will be incorrect. However, this
commit aims only to fix the (unreleased) behavior of showing subprocess
output as ephemeral messages inside open_stream().
  • Loading branch information
tigarmo authored Jun 14, 2023
1 parent 3fe790e commit 245f3b6
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 11 deletions.
3 changes: 3 additions & 0 deletions craft_cli/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ def _write(self, data: bytes) -> None:

# write the useful line to intended outputs
unicode_line = useful_line.decode("utf8")
# replace tabs with a set number of spaces so that the printer
# can correctly count the characters.
unicode_line = unicode_line.replace("\t", " ")
text = f":: {unicode_line}"
self.printer.show(self.stream, text, **self.printer_flags)

Expand Down
54 changes: 43 additions & 11 deletions examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,11 +382,26 @@ def example_26():
emit.message("Application End.")


def example_27(mode_name, total_messages=10):
def _run_noisy_subprocess(mode_name, total_messages, subprocess_code):
"""Capture the output of a noisy subprocess in different modes."""
mode = EmitterMode[mode_name.upper()]
emit.set_mode(mode)

temp_fh, temp_name = tempfile.mkstemp()
with open(temp_fh, "wt", encoding="utf8") as fh:
fh.write(subprocess_code)

emit.progress("About to run a noisy subprocess")
time.sleep(1)
with emit.open_stream("Running custom Python app in unbuffered mode") as stream:
cmd = [sys.executable, "-u", temp_name, str(total_messages)]
subprocess.check_call(cmd, stdout=stream, stderr=stream)
os.unlink(temp_name)
emit.message("All done!")


def example_27(mode_name, total_messages=10):
"""Capture the output of a noisy subprocess in different modes."""
example_test_sub_app = textwrap.dedent(
"""
import sys
Expand All @@ -406,17 +421,34 @@ def example_27(mode_name, total_messages=10):
time.sleep(5)
"""
)
temp_fh, temp_name = tempfile.mkstemp()
with open(temp_fh, "wt", encoding="utf8") as fh:
fh.write(example_test_sub_app)
_run_noisy_subprocess(mode_name, total_messages, example_test_sub_app)

emit.progress("About to run a noisy subprocess")
time.sleep(1)
with emit.open_stream("Running custom Python app in unbuffered mode") as stream:
cmd = [sys.executable, "-u", temp_name, str(total_messages)]
subprocess.run(cmd, stdout=stream, stderr=stream)
os.unlink(temp_name)
emit.message("All done!")

def example_28(mode_name, total_messages=10):
"""Capture the multi-line, tab-containing output of a noisy subprocess."""
example_test_sub_app = textwrap.dedent(
"""
import sys
import time
import textwrap
total = int(sys.argv[1])
for idx in range(total):
#short = random() > .2
delay = 1
delay_for_spinner = False # random() > .9
message = textwrap.dedent(f'''
This first message should never appear.
\tThis second message shouldn't appear either.
\tThis line should appear, preceded ":: " ({idx} / {total}).
''').strip()
print(message,flush=True)
time.sleep(delay)
if delay_for_spinner:
time.sleep(5)
"""
)
_run_noisy_subprocess(mode_name, total_messages, example_test_sub_app)


# -- end of test cases
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/test_messages_stream_cm.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,19 @@ def test_pipereader_ephemeral(recording_printer, stream):
assert msg.bar_total is None


@pytest.mark.parametrize("stream", [sys.stdout, sys.stderr])
def test_pipereader_tabs(recording_printer, stream):
"""Check that tabs are converted to spaces."""
flags = {"use_timestamp": False, "ephemeral": False, "end_line": True}
prt = _PipeReaderThread(recording_printer, stream, flags)
prt.start()
os.write(prt.write_pipe, b"\t123\t456\n")
prt.stop()

(msg,) = recording_printer.written_terminal_lines # pylint: disable=unbalanced-tuple-unpacking
assert msg.text == ":: 123 456" # tabs expanded into 2 spaces


def test_pipereader_chunk_assembler(recording_printer, monkeypatch):
"""Converts ok arbitrary chunks to lines."""
monkeypatch.setattr(messages, "_PIPE_READER_CHUNK_SIZE", 5)
Expand Down

0 comments on commit 245f3b6

Please sign in to comment.