Uh oh!
There was an error while loading. Please reload this page.
Make Shell.clear() safe for non-TTY stdout - #37
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Shell.clear() to avoid emitting ncurses/terminfo errors when clear is executed in non-interactive environments (piped/redirected output, CI, stripped environments), by gating the clear invocation on whether stdout is a TTY and adding a fallback behavior.
Changes:
- Update
Shell.clear()docstring to document interactive vs non-interactive behavior. - Call
os.system("clear")only whensys.stdout.isatty()is true. - For non-TTY stdout, write the ANSI RIS escape sequence (
\033c) and flush.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| redirected, ``sudo`` with a stripped environment, CI, etc.), fall | ||
| back to the ANSI RIS escape sequence (``\\033c``) so the call is | ||
| still harmless and doesn't trigger ncurses' ``'unknown': I need | ||
| something more specific.`` error when ``TERM`` is missing or set | ||
| to a value the local terminfo doesn't know. | ||
| """ | ||
| if sys.stdout.isatty(): | ||
| os.system("clear") | ||
| else: | ||
| sys.stdout.write("\033c") | ||
| sys.stdout.flush() |
There was a problem hiding this comment.
Good catch. Updated to make the non-TTY branch a plain no-op instead of writing the RIS escape into redirected stdout. Clearing has no meaning when there is no screen to clear, so we just return early.
| On an interactive TTY, defer to the ``clear`` binary so the user | ||
| gets a real terminal reset (including scrollback flush where the | ||
| emulator supports it). When stdout is not a TTY (output piped or | ||
| redirected, ``sudo`` with a stripped environment, CI, etc.), fall | ||
| back to the ANSI RIS escape sequence (``\\033c``) so the call is | ||
| still harmless and doesn't trigger ncurses' ``'unknown': I need | ||
| something more specific.`` error when ``TERM`` is missing or set | ||
| to a value the local terminfo doesn't know. | ||
| """ | ||
| if sys.stdout.isatty(): | ||
| os.system("clear") | ||
| else: | ||
| sys.stdout.write("\033c") | ||
| sys.stdout.flush() | ||
There was a problem hiding this comment.
Right — also addressed in the updated patch. Even on a real TTY, if TERM is empty, dumb, or unknown (e.g. sudo env -i), we now skip os.system("clear") entirely. So clear only runs when both stdout is a TTY and TERM looks usable; otherwise it's a no-op and no ncurses error can escape.
fd50d84 to
463133eCompare
When adafruit_shell.Shell.clear() is invoked from a script whose stdout
is not a real terminal (output piped or redirected, sudo with a
stripped environment, CI, etc.), the unconditional os.system("clear")
call falls through to ncurses' clear binary with a TERM that the local
terminfo database can't resolve. On Debian Trixie this surfaces as a
visible cosmetic error at the top of installer output:
'unknown': I need something more specific.
Treat "clear the screen" as a no-op when there's effectively no screen:
* stdout is not a TTY (piped/redirected/CI/agent harness), or
* TERM is unset / 'dumb' / 'unknown' (sudo with stripped env, env -i,
etc.), even on a real TTY.
In both cases there is nothing meaningful to clear, so we skip the
os.system("clear") call entirely. On an interactive TTY with a usable
TERM, behavior is unchanged: full terminal reset via the clear binary.
This also avoids writing control bytes into redirected stdout, which an
earlier draft of this fix would have done.463133e to
1cbf428Compare| When stdout isn't a TTY (output piped or redirected, CI, etc.) | ||
| or ``TERM`` is missing/unknown (``sudo`` with a stripped | ||
| environment, ``env -i``, ...), do nothing. Clearing has no | ||
| meaning when there's no screen to clear, and unconditionally | ||
| shelling out to ``clear`` in those cases prints ncurses' | ||
| ``'unknown': I need something more specific.`` to stderr. | ||
| """ | ||
| if not sys.stdout.isatty(): | ||
| return | ||
| term = os.environ.get("TERM", "") | ||
| if not term or term in {"dumb", "unknown"}: | ||
| return |
There was a problem hiding this comment.
Good catch — PR description updated to match the current implementation (no-op on non-TTY / unusable TERM, no RIS fallback).
Uh oh!
There was an error while loading. Please reload this page.
Summary
Shell.clear()currently callsos.system("clear")unconditionally. When thescript is invoked from a non-interactive context — output piped or redirected,
sudowith a stripped environment, CI runners, an automation/agent harness,etc. — ncurses'
clearbinary runs with aTERMvalue that its terminfodatabase can't resolve, and prints a visible cosmetic error to stderr:
That line shows up at the top of installer output for anyone running the
Adafruit installers (e.g.
adafruit-pitft.py) non-interactively on DebianTrixie. It's harmless but confusing — it looks like a real error.
Fix
Treat "clear the screen" as a no-op when there is effectively no screen:
TERMis missing,dumb, orunknown(sudo env -i,sudowith astripped environment, etc.) even on a real TTY → return early.
TERM→ behavior unchanged: shell out toclearso the user gets a real terminal reset (including scrollback flushwhere the emulator supports it).
An earlier revision of this PR fell back to emitting the ANSI RIS escape
sequence (
\033c) on the non-TTY path. That was dropped after review feedbackpointed out it would write control bytes into captured/redirected output. A
no-op is the right answer: there is no screen to clear, so we just don't.
Testing
Verified locally on Trixie with a small driver that exercises all four guard
branches (monkeypatching
sys.stdout.isattyandos.environ["TERM"]):TERMos.systemdumbunknownxterm-256coloros.system("clear")(original behavior)After applying the fix, the spurious
'unknown': I need something more specific.message no longer appears at thetop of installer output when running under non-interactive
sudo.