Skip to content

test: evaluate upstream #79 blank-line regression coverage - #93

Closed
denny0223 wants to merge 1 commit into
mainfrom
review/upstream-79
Closed

denny0223 wants to merge 1 commit into
mainfrom
review/upstream-79

Conversation

@denny0223

Copy link
Copy Markdown
Owner

This is an evaluation of upstream #79, whose indented-code fix is valid for the upstream renderer. The fork already preserves the blank line after indented code through 1d8538c and already tests that behavior.

The candidate copies tests/indented_code_test.lua unchanged from upstream a10f025. It contains no runtime changes and is intended to close without merging, after recording the assessment in a closing comment. It is a tests-only extraction, not a full cherry-pick.

The fork handles literal code earlier in the rendering loop and retains heading, thematic-break, and list resets alongside blank-state tracking. Copying upstream's runtime hunk would not add the missing behavior here. Keeping the established fork path avoids unnecessary state-handling churn; keeping the existing regression cases avoids duplicate permanent coverage.

Validation against fork base 94112ca, with Neovim 0.12.0 and 0.12.5:

  • Unchanged upstream tests: 2 passed, 0 failed.
  • Existing code_isolation_test.lua: 829 passed, 0 failed.
  • In an isolated copy, deleting only prev_rendered_blank = code_content:match "^[ \t]*$" ~= nil from the literal indented-code path: 825 passed, 4 expected failures. The failures are output rows and source ownership for the CM117 and wider-list cases; the unchanged candidate still passes all 829 assertions.
  • The copied test matches the upstream file byte for byte; StyLua 2.5.2 and Luacheck 1.2.0 pass.

The upstream tests were also run on upstream #79's parent and resulting commit under Neovim 0.12.0: 0/2 passing before, 2/2 passing after. This distinguishes a useful upstream fix from an unnecessary backport for this fork.

Extract only the unchanged upstream regression tests for an evaluation PR.
The runtime hunk is omitted because this fork already tracks indented-code
blank state in 1d8538c, together with its
heading, thematic-break, and list resets.

This candidate records the assessment; it is intended to close without
merging duplicate regression coverage into main.

Upstream-PR: delphinus#79
Upstream-commit: a10f025
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@denny0223

Copy link
Copy Markdown
Owner Author

Evaluated and closing without merge. The upstream fix is valid; this fork already implements the behavior and has regression protection, so neither a runtime backport nor duplicate permanent tests are needed.

Sources and scope:

The following commands passed on the candidate with both Neovim 0.12.0 and 0.12.5:

nvim -n -i NONE --headless -u NONE --noplugin -l tests/indented_code_test.lua
# 2 passed, 0 failed
nvim -n -i NONE --headless -u NONE --noplugin -l tests/code_isolation_test.lua
# 829 passed, 0 failed
stylua --check tests/indented_code_test.lua
luacheck --no-cache tests/indented_code_test.lua

To verify that existing coverage detects the regression, an isolated copy removed only prev_rendered_blank = code_content:match "^[ \t]*$" ~= nil from that path. code_isolation_test.lua then reported 825 passed and 4 expected failures on both versions: exact output rows and source ownership for CM117 and the wider-list case. The unchanged candidate was rerun and still passed 829/829. This mutation is not the same as setting the state to false or removing the fork's HR/list resets.

Separately, upstream's original two assertions failed on the parent of a10f025 and passed on that commit under Neovim 0.12.0. The upstream patch addresses a real bug; the decision here is about whether this fork needs the same change.

Revisit if the literal-code state handling is refactored, the cited regression coverage is removed, or a new failing case shows that the fork no longer preserves this behavior. No changes from this evaluation PR enter main.

@denny0223 denny0223 closed this Oct 26, 2026
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.

2 participants