Skip to content

Conversation

@LoricAndre
Copy link
Contributor

@LoricAndre LoricAndre commented May 24, 2025

Checklist

check the box if it is not applicable to your changes

  • I have updated the README with the necessary documentation
  • I have added unit tests
  • I have added end-to-end tests
  • I have linked all related issues or PRs

Description of the changes

@LoricAndre LoricAndre requested a review from Copilot May 28, 2025 21:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures terminal attributes are reset on exit in the TUI backend and adds coverage through an ANSI-focused end-to-end test, along with a new example for skim.

  • Reset terminal attributes after exiting TermLock to avoid leftover styling.
  • Add reset_after_exit_height e2e test and extend TmuxController to capture ANSI output with a configurable timeout.
  • Introduce a default.rs example demonstrating basic skim usage.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
skim/examples/default.rs New CLI example illustrating basic skim usage
skim-tuikit/src/term.rs Added output.reset_attributes() after flushing on exit
e2e/tests/ansi.rs New e2e test to verify ANSI attribute reset after exit
e2e/src/lib.rs Defined TIMEOUT, refactored wait, and added ANSI capture
Comments suppressed due to low confidence (3)

e2e/src/lib.rs:17

  • Consider renaming TIMEOUT to TIMEOUT_SECS (or similar) to explicitly convey that this constant represents seconds, improving clarity when it's used in timing calculations.
const TIMEOUT: usize = 30; // seconds

e2e/tests/ansi.rs:21

  • This assertion only checks the foreground reset. To fully cover reset_attributes(), consider also verifying that the background and other style attributes are cleared (e.g., check for \u{1b}[49m).
assert!(lines[1].contains("\u{1b}[39m2") || lines[1].contains("\u{1b}[0m2"));

skim/examples/default.rs:1

  • In Rust 2018 edition and later, extern crate skim; is redundant—consider removing it to align with modern conventions.
extern crate skim;

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.

Terminal colors are not restored after Skim::run_with returns

2 participants