Skip to content

Commit 846de16

Browse files
Treat \r as logical-line boundary and swallow stray padding spaces
Follow-up to #32. While that fix preserved \r-based progress updates end-to-end, two cosmetic glitches remained when running real apt / pip installs through run_command: 1. apt's progress UI sometimes emits a stray padding space after a \n, e.g. 'Fetched 52.4 MB in 30s (1,729 kB/s)\n Selecting...'. The previous logic treated the trailing space as the start of a fresh logical line, wrote the group prefix, then the space, then the real content -- producing output like: PITFT Fetched 52.4 MB in 30s (1,729 kB/s) PITFT Selecting previously unselected package ... ^ stray space, prefix pushed one column right 2. A bare \r-only line (e.g. apt's 'erase-progress-line' sequence) was being treated as 'still on the same logical line', so the next chunk's content would not get a prefix at all -- producing: PITFT 52.4 MB ... Selecting previously unselected package ... ^ no PITFT prefix; line just looks orphaned Reproduced both with a deterministic harness. Same root cause: the splitter only recognized \n as a logical-line boundary, and the emitter treated any non-empty body at line-start as 'real content' (so apt's padding-space pushed the prefix right). This change: * Splits chunks on either \n or runs of \r (\r+), so cursor-at-col-0 is the actual boundary, not just newline. * Strips leading horizontal whitespace (spaces and tabs) immediately after a boundary, so padding from the source process doesn't push the prefix right. * Suppresses the prefix entirely when a logical line is pure padding, but keeps the terminator (the \r or \n) so the terminal still sees cursor motion. * Stays in line-start state when an unterminated tail was pure padding, so the next chunk's real content still gets a prefix. Pip-style \r-only progress bars now correctly keep the prefix on every redraw ('PITFT 10%\rPITFT 25%\rPITFT 50%\r...') rather than having the prefix appear only on the very first frame. Verified end-to-end via a deterministic harness that simulates terminal CR/LF cursor semantics against captured output. Six scenarios pass: plain multi-line, \r progress, line-split-across- reads, apt stray-space, apt \r-pad-\r, pip \r-only progress, and the no-group raw passthrough. ruff check + ruff format clean.
1 parent 02d9c27 commit 846de16

1 file changed

Lines changed: 82 additions & 14 deletions

File tree

adafruit_shell.py

Lines changed: 82 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,35 @@ def preexec():
190190
return False
191191
return True
192192

193+
# Match either ``\n`` (advance to next line) or one-or-more ``\r``
194+
# (return cursor to column 0). Both put the terminal at column 0, so
195+
# both are treated as logical-line boundaries for prefix re-emission.
196+
# We coalesce runs of ``\r`` so progress like ``\r\r%5d\r`` doesn't
197+
# cause us to write the prefix multiple times back-to-back.
198+
_LINE_BOUNDARY_RE = re.compile(r"\n|\r+")
199+
193200
def _emit_stream_chunk(self, chunk, *, kind, at_line_start):
194201
"""
195202
Write a chunk read from a subprocess stream to stdout, preserving the
196203
process's own line terminators (including ``\r``-only progress updates)
197204
and prepending the colored group prefix at the start of each logical
198205
new line.
199206
207+
"Logical line" here means "the cursor is at column 0", which is true
208+
after either ``\n`` (newline) or ``\r`` (carriage return). Both kinds
209+
of boundary trigger prefix re-emission so that in-place progress
210+
updates like ``Downloading 12%\rDownloading 24%\r...`` keep showing
211+
the prefix as they redraw, and so that any content following a bare
212+
``\r`` (e.g. apt's "erase progress line, then start the next status
213+
line") is not mistakenly appended to the previous line's tail.
214+
215+
Leading horizontal whitespace right after a logical-line boundary is
216+
treated as padding (apt and pip emit space-padded "clear" sequences
217+
like ``\r \r`` to wipe a progress line) and is
218+
suppressed before the prefix is written, so output reads as e.g.
219+
``PITFT Selecting previously unselected package ...`` rather than
220+
``PITFT Selecting ...`` with stray indentation.
221+
200222
``kind`` selects the color used for the group prefix
201223
(``"info"`` -> green, ``"error"`` -> red). The ``end="\n\r"`` that the
202224
old code hardcoded is *not* added here; whatever terminators the
@@ -220,23 +242,69 @@ def _emit_stream_chunk(self, chunk, *, kind, at_line_start):
220242

221243
prefix = colorize(self._group) + " " if self._group is not None else ""
222244

223-
# Split on '\n' but keep the separator attached to each segment so
224-
# we can detect logical line boundaries. A bare '\r' inside a segment
225-
# is *not* a new logical line -- it's an in-place update of the
226-
# current line -- so we deliberately don't re-emit the prefix for it.
227-
parts = chunk.split("\n")
228-
for index, part in enumerate(parts):
229-
is_last = index == len(parts) - 1
230-
segment = part if is_last else part + "\n"
231-
if not segment:
232-
continue
233-
if at_line_start and prefix:
234-
stream.write(prefix)
235-
stream.write(segment)
236-
at_line_start = segment.endswith("\n")
245+
# Walk the chunk segment-by-segment, where each segment is the run of
246+
# bytes between two consecutive line boundaries (``\n`` or ``\r+``).
247+
# The boundary itself is written after its preceding segment, and the
248+
# next segment is treated as the start of a fresh logical line.
249+
pos = 0
250+
for match in self._LINE_BOUNDARY_RE.finditer(chunk):
251+
body = chunk[pos : match.start()]
252+
boundary = match.group(0)
253+
self._write_logical_line(stream, prefix, body, at_line_start, terminator=boundary)
254+
at_line_start = True
255+
pos = match.end()
256+
# Anything after the last boundary is an unterminated tail; emit it
257+
# with no terminator. If we were at a line start and the tail was
258+
# pure leading whitespace that we suppressed, stay at line-start so
259+
# the prefix gets emitted with the real content on the next chunk.
260+
tail = chunk[pos:]
261+
if tail:
262+
wrote = self._write_logical_line(stream, prefix, tail, at_line_start, terminator="")
263+
if wrote:
264+
at_line_start = False
265+
# else: nothing was actually written (pure padding swallowed);
266+
# leave ``at_line_start`` as-is so the next chunk still gets
267+
# the prefix on its first real content.
237268
stream.flush()
238269
return at_line_start
239270

271+
@staticmethod
272+
def _write_logical_line(stream, prefix, body, at_line_start, *, terminator):
273+
"""Write one segment (optionally prefixed, optionally terminated).
274+
275+
If ``at_line_start`` is true and a prefix is configured, the prefix
276+
is written first, then ``body`` with any leading horizontal
277+
whitespace stripped (so apt/pip padding doesn't push the real content
278+
to the right). If ``body`` is empty after stripping, the prefix is
279+
still suppressed so we don't leave a dangling ``PITFT `` on a
280+
whitespace-only "clear" line.
281+
282+
Returns True if any body bytes were written (i.e. real content
283+
landed on this logical line). The terminator is written regardless,
284+
but does not count as body content -- callers use the return value
285+
to decide whether to flip ``at_line_start``.
286+
"""
287+
wrote_body = False
288+
if at_line_start:
289+
# Strip leading horizontal whitespace (spaces/tabs) that the
290+
# source process used as padding. Don't strip ``\r`` / ``\n`` --
291+
# the regex already consumed those.
292+
stripped = body.lstrip(" \t")
293+
if stripped:
294+
if prefix:
295+
stream.write(prefix)
296+
stream.write(stripped)
297+
wrote_body = True
298+
# else: pure-whitespace segment, swallow it; the terminator
299+
# below (likely ``\r``) still gets written so the terminal
300+
# still sees the cursor return.
301+
elif body:
302+
stream.write(body)
303+
wrote_body = True
304+
if terminator:
305+
stream.write(terminator)
306+
return wrote_body
307+
240308
def write_templated_file(self, output_path, template, **kwargs):
241309
"""
242310
Use a template file and render it with the given context and write it to the specified path.

0 commit comments

Comments
 (0)