Skip to content

fix(messages): reset terminal style after open_stream - #444

Merged
bepri merged 5 commits into
canonical:mainfrom
gcomneno:contrib/issue-415-open-stream-reset-clean
Sep 1, 2026
Merged

fix(messages): reset terminal style after open_stream#444
bepri merged 5 commits into
canonical:mainfrom
gcomneno:contrib/issue-415-open-stream-reset-clean

Conversation

@gcomneno

Copy link
Copy Markdown
Contributor

Summary

Reset terminal style after open_stream() closes.

If output written through open_stream() includes ANSI control sequences that alter terminal styling, that styling could affect subsequent output because no reset sequence was emitted when the stream context manager exited.

This change adds a terminal-style reset on stream close through the printer layer, and includes focused unit and integration coverage.

Testing

pytest -q tests/unit/test_messages_stream_cm.py -rs
pytest -q tests/integration/test_messages_integration.py -k "third_party_output_brief_terminal or open_stream_no_text" -rs
ruff check craft_cli/messages.py craft_cli/printer.py tests/unit/test_messages_stream_cm.py tests/integration/test_messages_integration.py

@gcomneno
gcomneno requested a review from tigarmo as a code owner April 23, 2026 03:40
@gcomneno

Copy link
Copy Markdown
Contributor Author

Fixed the CI regression: _safe_print and its suppress import were restored in craft_cli/printer.py. Re-ran the focused checks locally:

  • ruff check craft_cli/messages.py craft_cli/printer.py tests/unit/test_messages_stream_cm.py tests/integration/test_messages_integration.py
  • pytest -q tests/unit/test_messages_stream_cm.py -rs
  • pytest -q tests/integration/test_messages_integration.py -k "third_party_output_brief_terminal or open_stream_no_text or streaming_brief_spinner" -rs -vv

@gcomneno

Copy link
Copy Markdown
Contributor Author

Hi! Just gently checking whether there’s anything I should adjust here.

I rechecked this locally against the current upstream/main and re-ran the targeted checks:

  • uv run --group dev python -m pytest -q tests/unit/test_messages_stream_cm.py -rs
  • uv run --group dev python -m pytest -q tests/integration/test_messages_integration.py -k "third_party_output_brief_terminal or open_stream_no_text or streaming_brief_spinner" -rs -vv
  • uv run --group lint --group types ruff check craft_cli/messages.py craft_cli/printer.py tests/unit/test_messages_stream_cm.py tests/integration/test_messages_integration.py
  • uv run --group lint --group types ruff format --check craft_cli/messages.py craft_cli/printer.py tests/unit/test_messages_stream_cm.py tests/integration/test_messages_integration.py

The remaining CI failures still look unrelated to this PR’s diff: the failing ty diagnostics are outside the files touched here, while the targeted tests and Ruff checks pass.

Happy to rework this if useful, or to close it if this should not move forward.

@gcomneno

Copy link
Copy Markdown
Contributor Author

I rechecked the latest QA failure after commit e00a189.

The previous ty diagnostic in craft_cli/messages.py is now fixed. The remaining lint / files failure is still from ty check, but the diagnostics are outside this PR’s diff:

  • craft_cli/errors.py:122
  • tests/unit/test_help.py:1260
  • tests/unit/test_help.py:1314
  • tests/unit/test_help.py:1402
  • tests/unit/test_help.py:1443

The touched files for this PR remain focused on stream/message handling, and the targeted checks passed locally before the latest push.

@bepri bepri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just one comment on the tests, otherwise LGTM! Thanks :)

Comment thread tests/unit/test_messages_stream_cm.py
@bepri
bepri requested a review from a team August 31, 2026 12:24

@lengau lengau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@bepri
bepri merged commit 2253735 into canonical:main Sep 1, 2026
31 checks passed
@gcomneno
gcomneno deleted the contrib/issue-415-open-stream-reset-clean branch September 1, 2026 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

open_stream() allows control characters through the stream that affect the full output

3 participants