Skip to content

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

Open
gcomneno wants to merge 3 commits into
canonical:mainfrom
gcomneno:contrib/issue-415-open-stream-reset-clean
Open

fix(messages): reset terminal style after open_stream#444
gcomneno wants to merge 3 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.

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.

1 participant