Repository navigation
test(diff): fold the trailing-blank-line spec into the final-line-break spec - #571
Conversation
…ak spec diff_accept_trailing_newlines_spec.lua (#535) copied the setup of diff_accept_missing_final_newline_spec.lua, and 12 of its 28 cases repeated cases of that spec. Only two rows were new: the proposal "a\n\nb\n\n" and the old file "a\n". Both rows move into the tables of the final-line-break spec, which runs them in all three layouts, and the duplicate file goes. Every assertion stays: one answer, FILE_SAVED, and the accepted text equal to the proposal. With the fix of #535 reverted, the old trailing spec failed 8 of 28 cases and the final-line-break spec failed 9 of 94. The merged spec passes 136 of 136 at head, and with the fix reverted it fails the 16 blank-line cases of the unified layout, among them every case that either old spec failed.
Only a proposal that ends in "\n\n" lost one. A proposal that ends in a single "\n" did not, so "ends in blank lines" was too wide.
|
@codex review |
|
@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. Bravo. 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. |
Why
Two integration specs ran the same test for the accepted text of a diff.
diff_accept_trailing_newlines_spec.lua(88 lines, from #535) opens a diff, accepts it unedited, and compares the answer with the proposal.diff_accept_missing_final_newline_spec.lua(from #542) does the same over a larger table of layouts, old files and proposals. Both had the same setup and the same four assertions. Twelve of the 28 cases of the trailing spec were also rows of the other one. A second copy of the loop means two places to fix when the accept path changes.What changed
Only two files change, both under
tests/integration/. No file underlua/orplugin/changes.diff_accept_missing_final_newline_spec.luagets two rows. These were the only cases that the trailing spec ran and the table did not:"a\n\nb\n\n"("a blank line inside and one after the last line"), and"a\n"("a file that is a line shorter").diff_accept_trailing_newlines_spec.luais deleted."\n\n"lost one ("a\n\n"came back as"a\n"). A proposal that ends in a single"\n", such as"a\n"or"\n", kept it.The merged spec has 136 tests: 3 layouts × 4 old files × 11 proposals, plus the 4 edit tests that were already there. Before, the two specs had 94 + 28 = 122 tests.
Nothing is lost
call._deferred), exactly one answer, the answer isFILE_SAVED, and the accepted text equals the proposal.git grepfor its name, its describe title andtrailing_newlines_spec: 0 hits at the head and onorigin/main).Verification: the merged spec still catches the bug of #535
I reverted #535 (the reverse patch of
884f7e22onlua/claudecode/diff_inline.lua) and ran each spec alone on a copy of the head.(The first two rows ran the two specs of the base of this PR,
37ee1e35, on Neovim 0.12.3 and 0.8.0, with the same counts on both. The merged spec's row ran on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0, with the same counts on all five.)"\n\n":"a\n\n","a\n\n\n","\n\n"and"a\n\nb\n\n". Proposals that end in a single"\n"pass with fix(diff): keep every trailing blank line in an accepted unified diff #535 reverted, as they did before fix(diff): keep every trailing blank line in an accepted unified diff #535."a\n\nb\n\n"against"old"(a file without a final line break) and against a file that does not exist. They come from the new proposal row.Gates
The head is
1f736d0d95154574376390c92f55bbcf3ad308a3. It mergesorigin/mainat5c7a4db9. The full gates ran at76b50be9, the merge before it. The commit after it brings onlyterminal.lua,scripts/, docs and the newtests/integration/terminal_cwd_spec.luafrom main, and nothing in the two specs of this PR. At the head I ranformat-check,check.sh --hash-onlyof all four skills, and the merged spec on Neovim 0.12.3 and 0.8.0 (136 ok each).mise run all(BUSTED_SEED=97531) exits 0: 1832 successes, 0 failures, 0 errors, 29 pending. luacheck: 0 warnings, 0 errors in 189 files.mise run format-checkexits 0 at both heads (432 files, 0 changed at the last one).scripts/run_integration_tests_individually.shexits 0 on Neovim 0.12.3 (Total Success 614, Failed 0, Errors 0) and on Neovim 0.8.0 (Total Success 596, Failed 0, Errors 0).check.sh --hash-onlyofverify-diff-lifecycle,verify-mention-queue,verify-selection-trackingandverify-terminal-lifecycleexits 0, at both heads. The fullcheck.shofverify-diff-lifecycleexits 0 (main93477 distinct states,live-DisconnectedCallsAnswered5557). The PR changes no covered file.One comment line of the table spec is 123 columns wide. It comes from #542 and I left it as it was.
Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh