Repository navigation
fix(diff): save an emptied proposal buffer as an empty string in the split layouts - #551
Merged
Merged
Conversation
…split layouts
A user who empties the proposal buffer (`ggdG` or `:%delete`) and saves it
with `:w` got "\n" in the answer, while Neovim's own `:w` of an emptied
buffer writes no bytes.
The vertical and horizontal layouts save the lines of the proposal buffer
and add a final line break when the buffer's 'eol' is on. A buffer with no
lines ("--No lines in buffer--") still gives one empty string through
nvim_buf_get_lines, so the save could not tell it from a buffer with one
empty line, and it added the line break.
Count the bytes of the buffer with wordcount() inside nvim_buf_call. It
is 0 for an empty buffer and at least 1 for a buffer with one empty line.
The save adds the line break only when the count is not 0. The mock gains
wordcount() and an `empty` flag, and nvim_buf_call names its buffer.
main now has the split-layout fix (#542) and the server and cwd fix (#539). In this merge: - covered.lock is re-recorded with check.sh --record. Only the diff.lua file hash and the resolve_saved block hash differ from main's lock. - One README bullet replaces this branch's bullet and #542's bullet about the saved content of the split layouts. It states both rules, 'eol' from the proposal and the empty buffer, and says that a buffer with one empty line is saved as "\n" only when 'eol' is on. - The no-edit controls of the new spec gain the empty proposal.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
This was referenced Nov 12, 2026
Merged
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
… other path characters (coder#561) ## Why `cwd.git_root` must find the repository of a folder whatever its name is. A repository in a folder with a space or a comma in its path was not found when git could not run: the fallback gave nil, or the repository of Neovim's current folder. A folder named `[x] {y}` or `$HOME-like` was lost or changed even when git answered. A verifier of F12 found the first case (finding F12-P1, a space or comma in the path makes the `.git` fallback return nil). ## What changed Four problems in `lua/claudecode/cwd.lua`. The first two took a path as a pattern. The third stepped one folder down at a trailing `..`. The fourth is a flaw in my first version of the second step. 1. **The `.git` fallback** called `vim.fn.finddir(".git", start_dir .. ";")`. `finddir` reads its path as a list in `'path'` option syntax. A space or a comma split the path, and a `;` ended it. The fallback now walks up from the full path of `start_dir` and tests each folder for a `.git` folder with `vim.fn.isdirectory`. A relative `start_dir` now reaches the folders above the current one too. Before, a relative start gave a wrong answer or nil in some cases (for example `git_root("sub/deeper")` in the repository root). 2. **`normalize_dir`** passed every path through `vim.fn.expand`. `expand` reads `[`, `{`, `*`, `?`, `$` and `\` in a name as patterns or variables. A folder named `[x] {y}` or `it's` came back with a trailing slash. A folder named `$HOME-like` or `back\slash` came back nil. This also happened when `git rev-parse` answered. `normalize_dir` now returns a directory that exists as written, and expands any other string (`~`, `$VAR`). See step 4 for text that starts with `~` or `$`. 3. **`..` before Neovim 0.11.** Before 0.11, `fnamemodify(path, ":p")` keeps a last `..`, and `:h` of `a/b/..` is `a/b`. So a walk from a start folder that ends in `..` stepped one folder down and returned the wrong root. The walk now calls `simplify()` on the full path first. A verifier found this. 4. **Text that starts with `~` or `$`.** The first version of step 2 took a path that names an existing folder as written, on the raw text. `vim.fn.isdirectory` does not expand, so with `cwd = "~"` or `"$HOME"` and a stray folder of that literal name in Neovim's current folder (for example one that an unexpanded `mkdir "~/x"` made), `git_root` and `resolve` returned the stray folder where main returned the home folder. `normalize_dir` now sends text that starts with `~` or `$` through `expand` first, as a shell and `:cd` do. A verifier of the terminal change found this. `lua/claudecode/README.md` gets one new bullet that states this, with the rule for text that starts with `~` or `$`. It sits after the "Terminal cwd precedence" bullet. It names `git_root` and `resolve` only, and it says that the terminal still passes a static `cwd` and a `cwd_provider` result through `vim.fn.expand` itself. ## Scope - Only `cwd.lua` changes in code. Steps 1 and 2 are one change of idea: a path is a literal, not a pattern. Step 3 is a quirk of `fnamemodify` before Neovim 0.11. Step 4 closes a hole that step 2 opened. - `normalize_dir` serves `cwd.git_root` and `cwd.resolve`. Nothing in `lua/` calls `cwd.resolve`. The terminal calls `cwd.git_root` when `git_repo_cwd` is set, so this PR fixes folder names with `$` and `\` on that path: when git does not answer, the terminal now gets `$HOME-like`, `back\slash`, `[x] {y}`, a space and a comma right, where main gave nil or the repository of Neovim's current folder. A static `cwd` and a `cwd_provider` result do not go through `normalize_dir`: `terminal.lua:build_config` passes them through `vim.fn.expand` itself, so this PR does not change what the terminal does with those. An absolute start path that ends in `..` is fixed here, through the `simplify()` call above. - Two gaps stay for separate follow-ups. Main already has both, and this PR does not reach them. First, the fallback finds a `.git` folder only, not a `.git` file in a worktree or submodule. Second, the walk is lexical, so a link into another repository answers with the repository of the link's own folder, where git answers with the link target's repository. The terminal's `expand` of a static `cwd` and of a provider result is a third follow-up, and it has its own change. - `~` and `$VAR` still expand in `cwd.resolve` (the spec checks both). The spec's cases for `cwd.resolve` are named as `cwd.resolve` cases, because they do not test the terminal. - `git rev-parse` still runs first. Its result for a plain path is the same string as before. ## Blast Radius - User-visible: a repository in a folder with a space, a comma, a quote or another path character is now found when git cannot run. With git, `git_root` returns a folder with `[`, `{`, `*`, `?`, `$` or `\` in its name exactly. It does not reach the terminal through a `cwd_provider`: `build_config` still expands the provider's result, so the root README's example still loses `$` and `\` there (follow-up). A plain path gives the same result as before. - The fallback result is an absolute path with no `..` part. Before it could be relative. The one exception is a relative start whose name begins with `~`: Vim reads `~` as the home folder and `~name` as a user's home folder, so `:p` leaves `~name` relative and the result can be the relative `~name`. That is a pathological case, and no caller in `lua/` passes a relative path (the terminal passes the file's folder from `expand("%:p")` or `getcwd()`). - A string that names an existing folder is taken as written, except text that starts with `~` or `$`. That text always goes through `expand` first, so a folder named `~` or `$HOME` in Neovim's current folder does not take the place of the home folder, as on main. The cost: a relative folder name that starts with `$` and reads as a variable (`$HOME-like`, `$odd/sub`) is expanded and not found, as on main. A lone relative `$odd` that names no variable is left unchanged by Vim and is still found. An absolute path never starts with `~` or `$`, so every folder name this PR fixes keeps working. The change applies to `git_root`'s `start_dir` and to `cwd.resolve`. The terminal's own `expand` of a static `cwd` and a provider result is a separate change. - Neovim 0.8: the code uses `vim.fn.isdirectory`, `vim.fn.fnamemodify` and `vim.fn.simplify`, which exist there. I ran the new spec on Neovim 0.8.0, 0.10.0 and 0.12.3. The `..` case fails on 0.8.0 and 0.10.0 without the `simplify()` call, and passes on 0.12.3 either way. - `covered.lock`: `cwd.lua` is the terminal's working-directory lookup. `check.sh` printed STALE for the file hash of `cwd.lua` only, with no changed block or call site. The change cannot reach diff state, an openDiff answer, a disconnect or a diff command. The model passed (`main`: 89205 distinct states, `live-DisconnectedCallsAnswered`: 5269). `check.sh --record` wrote the new lock and `check.sh --hash-only` reports that it matches. ### Merge with coder#539 (F12) coder#539 landed first. This PR's merge commit with `origin/main` resolves the interplay: it re-records `covered.lock` and changes the one stale test in coder#539's `cwd_spec.lua` (see Verification). F11 (coder#535) merges with this PR without a conflict. ## Verification Red before the fix, green after (same spec, only `lua/claudecode/cwd.lua` swapped): | Spec | origin/main | with the fix | | --- | --- | --- | | `tests/integration/cwd_path_characters_spec.lua` (37 cases, real Neovim, 0.8.0 and 0.12.3) | 20 successes, 17 failures | 37 successes, 0 failures | | the new case "goes up from a start folder that ends in .." at `ede2f733` (before the `simplify()` call) | fails on 0.8.0 and 0.10.0, passes on 0.12.3 | passes on 0.8.0, 0.10.0 and 0.12.3 (35 successes when added, 37 at the final head, 0 failures on each) | | the two cases "expands a leading ~ / $ even when Neovim's cwd holds a folder named ~ / $HOME", with `cwd.lua` from `ca9e61c0` (before the guard) | 35 successes, 2 failures on 0.8.0 and 0.12.3 | 37 successes, 0 failures on each | The spec makes real folders and runs `cwd.git_root` on them, in two modes. In the first mode git is off `PATH` (the shell keeps its absolute path), so `git` ends with "not found" and a list command raises E475, as on a host without git. In the second mode a real `git init` makes the repository. The folder names are: a space, a comma, a semicolon, single and double quotes, brackets and braces, parentheses, non-ASCII letters, a star and a question mark, a percent sign and a hash, a backslash, a dollar sign, a leading tilde, and a plain name. Each name runs from the repository folder and from a folder two levels below it, and the answer must equal the repository path with no trailing slash. A folder that is not a repository returns nil, whatever its name. Seven more cases cover a relative start folder, a start folder that ends in `..`, `cwd.resolve` with `git_repo_cwd`, a static `cwd` with brackets and a star in `cwd.resolve`, `~` and `$VAR` in a setting of `cwd.resolve`, and two cases where Neovim's current folder holds a folder named `~` or `$HOME` and the setting must still expand. The 17 red cases are 10 of the no-git names, 4 of the git names, the relative start, `resolve`, and the static `cwd`. Gates on the final head `f12368bd`, which contains a merge of `origin/main` (05b9785, with coder#551, coder#535, coder#494, coder#545, coder#553, coder#555 and coder#557) (private XDG folders). An independent verifier passed `c73ab044`. Since then the branch has three changes of its own and two merges with main. The changes are a docs commit (the README sentence about where the walk starts), the fix for text that starts with `~` or `$` (step 4, with two spec cases and the README clause), and the `covered.lock` record for it. For the lock I ran the full `check.sh`: the model passed (89205 distinct states, 5269 live states, every fact TRUE), and only the file hash of `cwd.lua` was stale, with no changed block or call site. - `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 186 files. busted: 1756 successes / 0 failures / 0 errors / 29 pending. treefmt: 0 files changed. Since coder#545, `mise run all` also runs the static proofs of the `verify-claudecode-nvim` skill, and they pass. - `mise run format-check`: native exit 0 (428 files, 0 changed). - `scripts/run_integration_tests_individually.sh` on Neovim 0.12.3: 564 successes, 0 failed, 0 errors, native exit 0. On Neovim 0.8.0: 427 successes, 0 failed, 0 errors, native exit 0 (the 0.8.0 run skips the specs that need 0.9). The new spec gives 37 successes on both. It starts about 100 processes (git and a shell), so it takes about 6 s on a quiet host and can pass the runner's 40 s limit at a load average above 100. A TIMEOUT line there means the host was slow, not that the spec hung. - All four `.agents/skills/verify-*/check.sh --hash-only` match their `covered.lock`. The merge with `origin/main` had two parts to resolve in an earlier commit. `covered.lock` conflicted on the hash line of `cwd.lua`, so I ran the full `check.sh` and `check.sh --record`. coder#539's `tests/unit/cwd_spec.lua` had a test that stubbed `vim.fn.finddir`, which the fallback no longer calls, so that test now walks up real folders. It does not depend on where `TMPDIR` is: it passes with `TMPDIR` inside a git checkout too. `cwd.lua` itself merged without a text conflict. The two latest merges needed no `covered.lock` change: coder#551 conflicted only in `tests/mocks/vim.lua`, where the branch keeps both its `simplify` entry and coder#551's `wordcount`, and coder#535 and coder#494 merged without a conflict. An independent verifier found the `..` case, the README wording and the other points that this body now states. The `terminal.lua` expand and `.git` files in worktrees are gaps that main already has. They are follow-ups, not part of this PR. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…oder#577) ## Why In the `vertical` and `horizontal` layouts, a proposal whose line breaks are all CRLF was loaded into the proposal buffer with `'fileformat'` `unix` and a CR at the end of each line. The save joined the lines with LF. So a line that the user typed, or the join of two lines, came back with LF, or with a CR in the middle of a line, where Neovim's `:w` writes CRLF. A CRLF proposal that equals a CRLF file also showed every line as changed, because the file's own buffer is `dos` and holds its lines without CRs. I found it while verifying coder#551. ## What changed 1. `_setup_blocking_diff` (`lua/claudecode/diff.lua`) loads a proposal as Neovim loads a file. When `'fileformats'` includes `dos`, and the proposal has a line break and only CRLF line breaks, the buffer gets `'fileformat'` `dos` and lines without their CRs. Any other proposal gets `'fileformat'` `unix` and is split at LF, as before. The diff sets `'fileformat'` in both cases, because a new buffer takes the first entry of `'fileformats'`, and that entry can be `dos`. 2. `_resolve_diff_as_saved` joins the lines with the line break of the buffer's `'fileformat'` (LF, CRLF for `dos`, CR for `mac`), and adds the final one when `'eol'` is on. Before, it always used LF. So `:setlocal fileformat=dos` on an LF proposal now answers CRLF, as `:w` writes it. 3. The `unified` layout does not change. Its buffer is read-only, and its answer is the proposal. 4. `lua/claudecode/README.md` states both rules, and that one empty line is saved as one line break of the buffer's `'fileformat'`. 5. `tests/integration/diff_accept_crlf_spec.lua` is new (159 tests). `tests/mocks/vim.lua` gives the mock `vim.o` the default `fileformats = "unix,dos"`, because the unit specs of the diff now read it. 6. `covered.lock` of `verify-diff-lifecycle` records the new hashes of `diff.lua` (the file, `resolve_saved` and `setup_blocking`). ## What a user sees 1. A line that the user types into an accepted CRLF proposal, or two lines that the user joins, comes back with CRLF, as `:w` writes it. For the proposal `"a\r\nb\r\n"` and a typed line after the last line, main answers `"a\r\nb\r\nx\n"` and this PR answers `"a\r\nb\r\nx\r\n"`. 2. A CRLF proposal that equals the CRLF file shows no difference. Main marked every line. 3. An unedited proposal still comes back as exactly the proposal, in every layout, under every `'fileformats'` value that I ran. 4. Nothing else changes for the default `'fileformats'` (`unix,dos`): the controls of the spec (an LF proposal, a proposal with CRLF and LF line breaks, a lone CR, no line break, unedited, emptied) give the same bytes on main and here. 5. `'fileformats'` `unix` (no `dos`) keeps a CRLF proposal as `unix` with its CRs, as the file's own buffer does. The equal-text view shows no difference there, and a typed line gets LF, as `:w` writes it. ## Verification The spec compares the answer with the bytes that Neovim's own `:w` writes for the same text after the same commands (an oracle that reads the text as a file under the same `'fileformats'`), or with an exact expected text. 1. Red, then green, run on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0 (all five give the same counts): 159 tests pass with the fix. With `origin/main`'s `diff.lua`, 105 pass and 54 fail. The unified cases run on 0.8.0 too (coder#564). 2. Fourteen mutants of the changed lines are each red on Neovim 0.12.3 (the last seven also on 0.8.0, with the same counts). Failed tests per mutant: - Split and join: the CRs stay in the lines 70, the join always uses LF 66, the final break is always LF 56, `mac` is ignored 2, dos for any text with a CRLF 12, dos for a text with no line break 2, dos even with a lone LF 32. - `'fileformats'` check: no check (always dos) 8, an exact match of `unix,dos` or `dos,unix` 4, dos for any value except `unix` 4, dos only when it is the first entry 42, `'fileformat'` always `unix` 64, always `dos` 48, set only for `dos` 12. 3. A probe of 1860 cases per run on Neovim 0.12.3 and 0.8.0 (three layouts, four old files, 15 proposals, 15 edits) compares each answer with the bytes of a fresh `nvim --clean` that reads the proposal as a file under the same `'fileformats'`, runs the same commands and writes with `:w`. The columns give mismatches. The two Neovim versions give the same numbers. | `'fileformats'` | main | this PR | unedited answers that differ from the proposal (main / this PR) | |---|---|---|---| | `unix,dos` | 748 | 144 | 0 / 0 | | `dos,unix` | 788 | 184 | 0 / 0 | | `unix` | 304 | 192 | 0 / 0 | | `dos` | 1104 | 516 | 0 / 0 | | `unix,mac` | 608 | 544 | 0 / 0 | Every mismatch that remains is one of the items under "Not fixed here", or a fold (see below). Cleanup leaves no state, buffer, window or tab behind in any case. Under `unix,dos`, `dos,unix` and `unix`, the equal-text view marks 0 lines in every case. 4. The only cases where this PR has a mismatch that main does not have are `dup_line` and `dd_first` (`yyp` and `dd` on line one) against an old file that equals the proposal, for CRLF proposals: 28 cases for each of `unix,dos`, `dos,unix` and `dos`. Diff mode folds every line of an equal text, so those commands act on the whole fold. Main does the same for an equal LF text (`same|lf|dup_line`), and showed every line of an equal CRLF text as changed, so it did not fold. This follows from item 2 of "What a user sees". 5. An independent verifier ran its own probe (2604 cases per run) under `unix,dos`, `dos,unix`, `unix` and `dos` on all five Neovim versions, and under `unix,dos,mac`, `unix,mac` and `dos,mac` on 0.12.3. In every run, 0 unedited answers differ from the proposal. ## Not fixed here These items are tracked in coder#581. All of them exist on `origin/main` already. An independent probe that compares each answer with `:w` of the same buffer found this PR never worse than main. 1. A CR inside a line, then `:setlocal ff=mac`. `:w` writes that CR as NL, and the save keeps the CR. The proposal `"a\rb\n"` gives `"a\rb\r"`, and `:w` gives `"a\nb\r"`. Main joined with LF, which was also wrong. 2. With `'fileformats'` `unix,mac`, Neovim reads a text such as `"a\r\nb\r\r\n"` as `mac`. The diff never detects `mac` (main neither). In 24 cases (an `ff=mac` edit only), main's answer equals the file-read oracle and this PR's answer does not, because main ignores `'fileformat'` when it joins. Against `:w` of the same buffer, both are wrong in these 24 cases. 3. An empty proposal, then typed lines. `'eol'` stays off (coder#542), so `Gox` gives `"\nx"`, where `:w` of an empty file gives `"\nx\n"`. 4. `'fileformats'` that starts with `dos`, and a proposal with no LF (`""`, `"a"`, `"a\r"`). Neovim reads it as `dos`. The diff keeps `unix`, so a typed line gets LF where `:w` writes CRLF. 5. `'fileformats'` with the single entry `dos`. Neovim then reads every file as `dos`, an LF file too. An LF or mixed proposal stays `unix`, so a typed line gets LF where `:w` of the file writes CRLF. Neovim reads the equal file as `dos`, so the equal-text view still marks lines for a text that is not all CRLF: this PR marks 2 of the 15 probe texts (a mixed text, and one with a leading lone LF), and main marks 11 of 15. ## Gates The head is `1c7b5adcfc165963ffc250ab9ebeedad00ddf954`. It merges `origin/main` at `7dbad04b`. Main moved three times while this PR was in progress, and each time only `covered.lock` conflicted. The gates below ran in full at `4525dad1` (the merge with `origin/main` at `c43b6a30`). The last merge brought coder#574. At the head I ran the checks of item 6, and CI runs the rest on this head. 1. `mise run all` (`BUSTED_SEED=97531`) exits 0: 1837 successes, 0 failures, 0 errors, 34 pending. luacheck: 0 warnings, 0 errors in 194 files. 2. `mise run format-check` exits 0. 3. `scripts/run_integration_tests_individually.sh` exits 0 on Neovim 0.12.3 (Total Success 828, Failed 0, Errors 0) and on Neovim 0.8.0 (Total Success 810, Failed 0, Errors 0). The new spec alone gives 159 passes on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0. 4. `check.sh --hash-only` of the four `verify-*` skills exits 0. The full `check.sh` of `verify-diff-lifecycle` exits 0 (`main` 93477 distinct states, `live-DisconnectedCallsAnswered` 5557). The merge with main conflicted in `covered.lock` only. I took my side, ran the full `check.sh` on the merged tree (all 23 facts TRUE, the model passes), and then ran `--record`. The last merge moved only `file` hashes (`diff.lua`, `init.lua`, `selection.lua`) and no block. Against `origin/main`, the lock moves 3 lines: the file hash of `diff.lua`, and the blocks `resolve_saved` and `setup_blocking`. No guard moved. The change decides which bytes the accepted text holds. It does not change a diff's status, who gets its answer, or when. 5. At the head, after the last merge: the full `check.sh` of `verify-diff-lifecycle` (all 23 facts TRUE, `main` 93477, `live-DisconnectedCallsAnswered` 5557) and then `--record`. coder#574 changed the blocks `open_blocking`, `handle_invoke` and `setup_blocking` on main, and the lock of this PR moves only the 3 lines of this PR against `origin/main` (the file hash of `diff.lua`, `resolve_saved`, `setup_blocking`). 6. At the head: `format-check` exits 0, `check.sh --hash-only` of the four skills exits 0, the new spec gives 159 passes on Neovim 0.12.3 and 0.8.0, and the 44 unit specs of the diff, tools and server modules give 752 successes, 0 failures. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
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.
Why
When a user empties the proposal buffer (
ggdGor:%delete) and saves it with:w, Claude must get what Neovim's own:wwrites. Neovim writes no bytes for an emptied buffer. With'eol'on, theverticalandhorizontallayouts answered" ", so Claude wrote a one-byte file where the user had asked for an empty one. The F14 verifier found this. PR #542 listed it as a known, pre-existing defect that this PR fixes.What changed
_resolve_diff_as_savedinlua/claudecode/diff.luasaves the lines of the proposal buffer, joined with line breaks. It adds a final line break when the buffer's'eol'is on. A buffer with no lines at all (Neovim shows "--No lines in buffer--") still gives one empty string throughnvim_buf_get_lines. So the save could not tell it from a buffer with one empty line, and it added the line break.The save now counts the bytes of the buffer with
vim.fn.wordcount().bytes, run insidenvim_buf_call. The count is 0 for an empty buffer and at least 1 for a buffer with one empty line when'eol'is on. The line break is added only when the count is not 0 and'eol'is on. That replaces the check#content_lines > 0, which was always true.I tried
nvim_buf_get_offsetandline2bytefirst. Both gave wrong sizes for a buffer thatggdGhad emptied (a stale line cache), so I did not use them.Other changes:
tests/mocks/vim.luagetswordcount()and anempty = trueflag for a mock buffer.nvim_buf_callin the mock now names the buffer that the callback acts on.tests/unit/diff_ui_cleanup_spec.luagets one fixture line. The mock'snvim_buf_callrefuses a buffer that does not exist, and that spec saved a buffer that the mock never had.lua/claudecode/README.md: one bullet replaces fix(diff): accept a proposal without a final line break as it is in the split layouts #542's bullet about the saved content of the split layouts. It states both rules,'eol'from the proposal and the empty buffer. It also says that a buffer with one empty line is saved as"\n"when'eol'is on and as""when it is off..agents/skills/verify-diff-lifecycle/covered.lockis re-recorded (see Blast Radius).Scope
verticalandhorizontallayouts share one code path, so one change fixes both.unifiedlayout is unchanged. It answers with the proposal, not with the buffer, so what the user does to its buffer never reaches Claude. The new spec runs it as a control.'eol'on, a buffer with one empty line is still saved as"\n". With'eol'off, it is saved as"", as before. A buffer with any line is saved as before.main(with fix(diff): accept a proposal without a final line break as it is in the split layouts #542), this PR changes the answer only for a buffer that the user emptied. It changes no answer for an unedited proposal: fix(diff): accept a proposal without a final line break as it is in the split layouts #542 already makes an empty proposal give"", and the new spec has that case as a control.Minor findings that I declined
Two verifiers (Opus and GPT) found these. None of them changes an answer that a user reaches with the default settings.
bomb(Opus F3). WithggdG, then:setlocal bomb, then:w, Neovim writes the 3-byte BOM. Bothmainand this PR answer without the BOM (mainanswers"\n"when'eol'is on and""otherwise, this PR answers""). The plugin never sets'bomb'on its scratch proposal buffer, and no answer of the plugin ever holds a BOM, so a user has to set a non-default option on a scratch buffer first. I left it.nvim_buf_callwrapper is covered by the mock spec only (Opus F4). If I delete the wrapper, only the mock unit spec fails. In the paths that save (BufWriteCmdandaccept_current_diff) the current buffer is always the proposal buffer, so the wrapper is defensive and no live answer changes. I kept the wrapper, becausewordcount()reads the current buffer, and I added no live test for a case that no path reaches.wordcount()(Opus F5). It adds a second pass over the buffer. For a 33 MB proposal that is about 0.1 to 0.2 s (0.12.3: +112 ms, 0.8.0: +216 ms), about the cost ofnvim_buf_get_linesplus the join. I accept that for one save.Blast Radius
wordcount()runs in the proposal buffer throughnvim_buf_call. It does not change the user's current buffer or window. Both functions exist in Neovim 0.8. I ran the specs on Neovim 0.12.3 only. The CI jobintegration-tests (v0.8.0)runs the new spec on 0.8.covered.lock: the change touches_resolve_diff_as_saved, a covered block.check.shprinted STALE for thediff.luafile hash and theresolve_savedblock hash only. It printed no changed call site, and all 21 code facts stay TRUE. The guard that lets only apendingdiff resolve is unchanged, and so are the status change tosavedand the one call of the resolution callback. The change only decides whether thetextstring of theFILE_SAVEDanswer ends in a line break. The model stores which kind of answer a call got, not its text, so no modelled transition changes. The model passed (main: 89205 distinct states,live-DisconnectedCallsAnswered: 5269).check.sh --recordwrote the new lock andcheck.sh --hash-onlyreports that it matches.Merge with #542, #539 and #535
#542 (F14) landed first, on 2026-11-12 at 04:36Z, and #539 (F12) at 04:46Z. #538 and #550 later changed
diff.luatoo, and #549 renamed "proposed buffer" to "proposal buffer" in the README. This PR mergedorigin/main(9bfc39c) with merge commits. It rancheck.sh --recordafter each change todiff.luaand kept main's README wording in the last one. Only thediff.luafile hash and theresolve_savedblock hash differ from the lock onmain. The fullcheck.shpasses andcheck.sh --hash-onlymatches.'eol'can be off.diff.luanor this README bullet. Its current head (a5dfc81b, withmainmerged) merges with this PR without a conflict.Verification
Red before the fix, green after (same specs, only
lua/claudecode/diff.luaswapped, on the merged head, so the red column ismainat 9bfc39c with #542):main(with #542)tests/unit/diff_saved_text_spec.lua(4 cases, mock)tests/integration/diff_accept_emptied_buffer_spec.lua(40 cases, real Neovim,:write)Against the older merge base (5acc100, before #542), the 38-case spec of round a had 24 red cases. Today's 40-case spec has 26 there: the 24 emptied cases plus the new control for an unedited empty proposal in both layouts. #542 fixes the 12 cases from proposals without a final line break (it sets
'eol'off), so 12 stay red onmain.The integration spec opens a real
openDiffand edits the proposal window. For theverticalandhorizontallayouts it runs 6 proposals (two lines and a final line break, one line and a final line break, one blank line, one line with no final line break, two lines with no final line break, and no text) with 2 ways to empty the buffer (ggdGand:%delete). Each case compares the answer with what Neovim's own:wwrites: the spec edits a real file buffer with the same text and commands, runs:write, and reads the file bytes, which are empty. Five controls per layout give the right text before and after (a blank line left in the buffer, no edit, an empty proposal without an edit, one line deleted, and text typed into the emptied buffer). In theunifiedlayout, 6 controls check that the answer is the proposal.Gates on the final commit, which merges
origin/main(9bfc39c) into the branch (private XDG folders):mise run all: native exit 0. luacheck: 0 warnings / 0 errors in 183 files. busted: 1710 successes / 0 failures / 0 errors / 26 pending. treefmt: 0 files changed.mise run format-check: native exit 0 (360 files, 0 changed).tests/unit/diff*_spec.lua,opendiff_blocking_spec,new_file_reject_then_reopen_spec, and theopen_diff_mcp,close_tabandclose_all_diff_tabsspecs): 226 successes, 0 failures.scripts/run_integration_tests_individually.sh: 457 successes, 0 failed, 0 errors. The new spec gives 40 successes..agents/skills/verify-diff-lifecycle/check.sh --hash-only: covered.lock matches the code.Two independent verifiers (Opus and GPT) checked the code at
ef8be297. Both found the code correct. Their blockers were the README sentence and the stale base, and this head fixes both.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh