Repository navigation
fix(cli): collect the changed cells once before painting a coloured diff - #601
Merged
Merged
Conversation
colored() asked the diff about every painted cell, which scanned the whole changed list each time: a full-screen change on a large terminal took seconds (vyncint#595). The changed cells now go into a HashSet first. A CLI test diffs two 1000x1000 screens that differ everywhere. Signed-off-by: berce <75228263+bercedev@users.noreply.github.com>
6 of 7 tasks
…t a sleep The new test was inserted between the vyncint#452 doc comment and the test it documents, so `diff_names_a_missing_or_unknown_color_value` lost its doc and the new test inherited it. It now sits above that comment. Its deadline was a try_wait loop with a 50 ms sleep, and CONTRIBUTING §3 allows only two sleeps in tests. A thread now waits on the exit and the test bounds that with recv_timeout. Checked against the old main.rs: the test still fails at the 60 s deadline. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> # Conflicts: # CHANGELOG.md
vyncint
approved these changes
Oct 22, 2026
vyncint
left a comment
vyncint
left a comment
Owner
There was a problem hiding this comment.
Thanks — the HashSet fix is exactly right. I pushed two small changes to the test: it had been inserted between the #452 doc comment and its test (so that test lost its doc), and its deadline used a try_wait + sleep loop, which CONTRIBUTING §3 doesn't allow; it now waits on the exit with recv_timeout. Checked against the old main.rs: the test still fails at the 60 s deadline. I also merged main to resolve the CHANGELOG conflict with #602.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Closes #595
colored()incrates/termlens-cli/src/main.rschecked every cell it painted withdiff.cells().any(...), which walks the whole changed list each time. A full-screen change was therefore quadratic. The changed(row, col)pairs now go into aHashSetonce, before the rows are painted, and the paint closure looks each cell up there. The output is byte-for-byte the same.A new CLI test,
diff_color_always_paints_a_full_screen_change_quickly, diffs two 1000x1000 screens that differ in every cell with--color always. It sends stdout to a file, because the output is tens of megabytes and a pipe nobody drains would stall the child. It also has a 60 s deadline, so a regression fails the test instead of hanging the suite. It checks for exit 1 and that both sides are painted (\x1b[31ma\x1b[0m,\x1b[32mb\x1b[0m). In a debug build it finishes in about 2.6 s with the change. With the oldmain.rsit hits the 60 s deadline.Checklist
cargo fmt --allandcargo clippy --workspace --all-targets --all-featuresare cleangit commit -s) — see CONTRIBUTING.md §5CHANGELOG.mdupdated under[Unreleased](user-facing changes only)cargo insta review, not blind-accepted (no snapshot changes)Checked with Rust 1.99.1, offline:
cargo test -p termlens-cli(44 passed),cargo clippywith all features and with default features,cargo fmt --all --check,.github/scripts/check-dco.sh main..HEAD,check-no-ai-attribution.sh main..HEADandcheck-readme-links.sh.