Fix failing editor and LSP tests - #7
Merged
sinelaw merged 3 commits intoNov 5, 2025
Merged
Conversation
Fixed issues: 1. Split.rs doctest - Mark Unicode box-drawing example as text 2. LSP diagnostics tests - Update to expect Background faces (refactoring changed from Underline to Background for better visibility) 3. Integration test - Update test_lsp_diagnostic_to_overlay to expect Background face 4. LSP async - Change blocking_send to try_send to work in async contexts Remaining issues (unclear if test or production code problem): - 2 LSP async tests expect synchronous initialization validation, but implementation queues commands asynchronously - Multiple e2e tests for movement, selection, and scrolling fail with cursor/buffer mismatches
Fixed bugs introduced in commit 0e0657c (action-to-events refactoring): 1. MoveDown - Was calling iter.next() once instead of twice - Old: iter.next(); iter.next() (skip current, get next) - New (buggy): iter.next() (only got current line) - Fixed: Restored double next() call 2. MoveLineStart - Was using current_position() instead of next() - Old: iter.next() to get line start - New (buggy): iter.current_position() - Fixed: Restored next() call 3. MoveLineEnd - Same issue, used current_position() + next() - Fixed: Use single next() call like original 4. SelectDown, SelectLineStart, SelectLineEnd - Same issues as Move variants - Fixed all with same pattern 5. SelectLine and ExpandSelection - Were excluded from action_to_events - These actions returned None instead of generating events - Fixed: Restored full implementations in actions.rs Test results: - Before: 25 e2e tests failing - After: 0 e2e tests failing (all movement and most selection tests pass) - Remaining: 2 LSP async tests (different issue - try_send vs blocking_send semantics) The line_iterator API returns current line on first next() call, not the next line. This was misunderstood during refactoring, causing cursor navigation bugs.
The ExpandSelection action was incorrectly selecting whole words. The original implementation selects from cursor position to end of word, not the entire word. Fixed behavior: - No selection: select from cursor to end of current word - If on non-word char or at word end: select to end of next word - Has selection: expand to end of next word (keep original anchor) This matches the behavior expected by all the test_expand_selection_* tests. All e2e tests now pass (103 passing). Only 2 LSP async tests remain failing (pre-existing issue).
sinelaw
pushed a commit
that referenced
this pull request
Apr 21, 2026
…ber cell
The PR-row renderer was passing the padded string
`pad(num, PR_COL_NUM)` to a single `ctx.text(..., { url, onClick })`
call. `emit` marks the whole span as clickable, which turns on the
underline for every cell — including the trailing padding spaces
the caller added to keep columns aligned. In a 140-col tmux capture
with `-e`, `#1641` came out as `[4m#1641 [0m` (underline covering
three whitespace cells past the digits).
Split the number and its padding into two separate `ctx.text`
calls: the first carries the URL + onClick (underline only on
"#N"), the second is plain whitespace that pads out the column.
Other clickable spans in the row (the title) were already padding-
free, so they're unaffected.
Verified by running fresh against a mock `gh` binary returning
three PRs with 1 / 4 / 6 digit numbers (#7, #1641, #363035) and
`tmux capture-pane -e`: `[4m...[0m` brackets now land exactly on
the digits and the trailing padding is rendered plain.
https://claude.ai/code/session_01DWRvL9UCe6XwNkLrDC5RRg
sinelaw
pushed a commit
that referenced
this pull request
Apr 21, 2026
…ber cell
The PR-row renderer was passing the padded string
`pad(num, PR_COL_NUM)` to a single `ctx.text(..., { url, onClick })`
call. `emit` marks the whole span as clickable, which turns on the
underline for every cell — including the trailing padding spaces
the caller added to keep columns aligned. In a 140-col tmux capture
with `-e`, `#1641` came out as `[4m#1641 [0m` (underline covering
three whitespace cells past the digits).
Split the number and its padding into two separate `ctx.text`
calls: the first carries the URL + onClick (underline only on
"#N"), the second is plain whitespace that pads out the column.
Other clickable spans in the row (the title) were already padding-
free, so they're unaffected.
Verified by running fresh against a mock `gh` binary returning
three PRs with 1 / 4 / 6 digit numbers (#7, #1641, #363035) and
`tmux capture-pane -e`: `[4m...[0m` brackets now land exactly on
the digits and the trailing padding is rendered plain.
https://claude.ai/code/session_01DWRvL9UCe6XwNkLrDC5RRg
sinelaw
pushed a commit
that referenced
this pull request
May 26, 2026
- Version: 0.3.9 (built from source) - Tests run: 12 objectives - Bug filed: #2122 — move_to_paragraph_down/up has no default keybinding - Settings UI checkboxes: navigable via DECCKM arrows + Enter (resolved Run 6 concern) - Dashboard no longer opens by default (0.3.9 confirmed) - Live Grep 0.3.9: scope toggles, provider cycle, Word/Regex modes all working - select_to_paragraph: confirmed via CSI 1;6B/1;6A escape sequences - confirm_quit: prompt format verified; letter+Enter required to confirm - Scroll sync: both splits synchronized correctly - Auto-revert: ~3s detection and revert of external file changes - Completion popup: setting toggles; popup requires active LSP - Lessons 29-34 added to learning_db.md
sinelaw
pushed a commit
that referenced
this pull request
May 26, 2026
…f false positive Tests completed: - TC-ALT-SLASH PASSED: Alt+/ opens Live Grep (0.3.8 feature) - TC-MARKDOWN PASSED: Markdown Compose mode with ANSI rendering confirmed - TC-MACRO-RECORD/PLAYBACK PASSED: F5 stop, F4 play, List Macros buffer - TC-SETTINGS-CTRL-R PARTIAL: Reset button reachable via Tab; Ctrl+R doesn't reset field - TC-REVIEW-DIFF-CONTROLS: FALSE POSITIVE corrected (Run #8 was wrong) Key discoveries: - Version is 0.3.8 (not 0.3.9 as logged in Runs #7-9) - Review Diff panel controls (n/d/q/s/u) are PLANNED but not implemented per docs/internal/review-diff-feature-restoration-plan.md — not a bug to file - DECCKM $'\033OB' must be unquoted in bash scripts - Explorer menu item appears contextually in menu bar 0 new bugs filed. https://claude.ai/code/session_01HNrE4L5AYRifpUJwQiyfAP
sinelaw
pushed a commit
that referenced
this pull request
Jun 8, 2026
…warning #7: LineAlignment::from_hunks paired old/new lines positionally within a hunk, so a pure insertion (e.g. comment lines above an unchanged block) mis-aligned — the unchanged block read as a delete on the left and a re-add on the right. Thread git's per-line ops (' '/'-'/'+') through CompositeHunk -> DiffHunk and build the alignment from them: context lines stay paired, deletions are old-only, insertions are new-only. Unchanged code now lines up identically on both sides. Falls back to the positional pairing when ops are absent. #2: composite source buffers appended a trailing newline to the last line, adding a phantom empty line with no ViewLine — 'ViewLine missing … line=N' spam when scrolled to the bottom. Build entries without the trailing newline so the buffer's line count matches the real lines. Regenerated fresh.d.ts for the new TsCompositeHunk.ops field. https://claude.ai/code/session_01D1vLAnwKHqmebN9qRQ9E47
sinelaw
pushed a commit
that referenced
this pull request
Jun 8, 2026
…warning #7: LineAlignment::from_hunks paired old/new lines positionally within a hunk, so a pure insertion (e.g. comment lines above an unchanged block) mis-aligned — the unchanged block read as a delete on the left and a re-add on the right. Thread git's per-line ops (' '/'-'/'+') through CompositeHunk -> DiffHunk and build the alignment from them: context lines stay paired, deletions are old-only, insertions are new-only. Unchanged code now lines up identically on both sides. Falls back to the positional pairing when ops are absent. #2: composite source buffers appended a trailing newline to the last line, adding a phantom empty line with no ViewLine — 'ViewLine missing … line=N' spam when scrolled to the bottom. Build entries without the trailing newline so the buffer's line count matches the real lines. Regenerated fresh.d.ts for the new TsCompositeHunk.ops field. https://claude.ai/code/session_01D1vLAnwKHqmebN9qRQ9E47
sinelaw
added a commit
that referenced
this pull request
Jul 1, 2026
…me history Addresses the remaining round-2 review findings. #3 — The Rust git-index BFS (app/git_index.rs) scanned one directory level deeper than the TypeScript discoverSubRepos it is documented to mirror, so a sub-repo exactly 4 levels below the workspace root got its index watched but was never decorated. Reframe the walk in terms of the level being scanned so both sides scan levels 1..=3, and spell out the shared contract in both docs. #6 — git_find_file listed every entry by its absolute path (long, shared prefix, worse fuzzy ranking) because label and open-path were the same string. Split into { rel, abs }: display/match the repo-relative path, open the absolute one. Also resolve the repo via resolveGitRepo so it works from a sub-project buffer when the workspace root isn't itself a repo. #7 — git_blame fetched historical content with `git show <rev>:<abs-path>`, which is fatal for an absolute path and fell through to the *current* working-tree content. Refer to the file as `<rev>:./<name>` from its own directory so git resolves it cwd-relative for any nesting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sinelaw
added a commit
that referenced
this pull request
Jul 7, 2026
…me history Addresses the remaining round-2 review findings. #3 — The Rust git-index BFS (app/git_index.rs) scanned one directory level deeper than the TypeScript discoverSubRepos it is documented to mirror, so a sub-repo exactly 4 levels below the workspace root got its index watched but was never decorated. Reframe the walk in terms of the level being scanned so both sides scan levels 1..=3, and spell out the shared contract in both docs. #6 — git_find_file listed every entry by its absolute path (long, shared prefix, worse fuzzy ranking) because label and open-path were the same string. Split into { rel, abs }: display/match the repo-relative path, open the absolute one. Also resolve the repo via resolveGitRepo so it works from a sub-project buffer when the workspace root isn't itself a repo. #7 — git_blame fetched historical content with `git show <rev>:<abs-path>`, which is fatal for an absolute path and fell through to the *current* working-tree content. Refer to the file as `<rev>:./<name>` from its own directory so git resolves it cwd-relative for any nesting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sinelaw
pushed a commit
that referenced
this pull request
Jul 26, 2026
…ghlighting claim Read audit_mode's implementation before planning its replacement, and corrected a wrong assumption in the process: - Correction: rendering the stream from a .diff file-backed buffer gives diff-level scoping only, NOT per-token language highlighting. The embedded-region mechanism (EMBEDDING_SPECS) is a build-time table keyed on host-grammar scopes and can't be driven by a plugin, and its language token is a language name, not a path. Per-token highlighting in a composed unified stream therefore needs one narrow, scheduled host addition (per-byte-range language regions reusing the engine's composite parse snapshots), now listed as primitive #7 and deferred to M2; M0 ships only the cheap add/remove backgrounds. - New section 4 records what must survive the migration: the single-pass build that emits rows, byte offsets, row-property maps and collapsible body ranges together; collapse as O(1) host fold registration with no rebuild; the plugin-side entryPropsByRow map (which exists to dodge a real getTextPropertiesAtCursor row-boundary bug); width-parameterised prebuild; the guarded sticky-header repaint; manual-refresh policy (auto-refresh on activation was removed for spawning git per panel switch); streamed SHA-cached git show. Plus the port-don't-rewrite list (staging engine with apply --cached and --check dry run, comment model and persistence, range/stash pipelines, repo resolution, i18n, e2e suites) and an explicit list of APIs considered and rejected because existing mechanisms cover them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01756Rh35n65zC4xHQcQAqBL
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
2 participants
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.
Fixed issues:
Remaining issues (unclear if test or production code problem):