[vim/vim] Fix concealed text wrapping at conceallevel 3 (PR #20472)
Samuel Roeca
fixes: #9443
Add regression coverage for concealed text that starts before the wrap column.
Caveat: I verified the logic and local tests on this Linux build, not Windows gvim specifically. The affected draw/redraw code path is shared, so this is still appropriate to mark as fixing #9443.
Known follow-up: screenline movement can still account for hidden columns. For example, with a long concealed prefix at 'conceallevel' 3, gj/gk may move through positions in the concealed text before reaching the next visible buffer line, even though the line no longer appears visually wrapped. I did not verify whether that behavior predates this PR, but it is separate from the wrapping/rendering bug fixed here.
This PR was prepared with assistance from OpenAI Codex.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20472
Commit Summary
- a9f0ddb Fix concealed text wrapping at conceallevel 3
File Changes
(5 files)
- M src/drawline.c (3)
- M src/screen.c (3)
- M src/testdir/dumps/Test_conceal_double_width_wrap_3.dump (4)
- M src/testdir/dumps/Test_conceal_double_width_wrap_4.dump (4)
- M src/testdir/test_conceal.vim (39)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
zeertzjq
zeertzjq left a comment (vim/vim#20472)Please see #10442 (comment). Neither cases there work properly in this PR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Samuel Roeca
pappasam left a comment (vim/vim#20472)Follow-up commits made the conceal wrap handling less special-casey and covered the cases the first patch missed: tabs after concealed text, visible text after concealed text, showbreak, virtual text, search/match conceal, syntax conceal edge cases, multibyte and wide characters, folded cursor lines, and screenpos() results. The final version adds conceal-aware line/column measurement in misc1.c, uses it from cursor and screenpos() code in move.c, and keeps the tab-specific wrap compensation narrow so ordinary visible text after concealed text does not double-count rows.
The tests now cover those regressions directly in test_conceal.vim and test_cursor_func.vim, including the final reviewer case where concealed text before visible wrapped text was making screenpos() report one row too far down.
Will start chipping away at CI/CD later tonight (if it keeps failing)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
zeertzjq
zeertzjq left a comment (vim/vim#20472)I think you need to update :h screenpos().
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Samuel Roeca
pappasam left a comment (vim/vim#20472)I think you need to update
:h screenpos().
docs have been updated!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Samuel Roeca
pappasam left a comment (vim/vim#20472)Performance notes:
This changes the cost model only for windows using conceallevel=3. The old line-height/cursor calculations measured display width directly. The new path must account for zero-width concealed text, so for cole=3 it may scan the line with syntax conceal, match conceal, tabs, multibyte characters, and textprop virtual text all considered.
Unaffected fast paths:
conceallevelother than 3- most non-conceal buffers
- non-wrapping line-height calculations
Potentially slower:
- long wrapped lines with
cole=3 - syntax or
matchadd()conceal on long lines - repeated cursor movement or
screenpos()calls on such lines
My belief: since this appears to be tightly scoped, I think the trade-off is acceptable. It fixes stale redraw/wrong cursor positioning for a configuration where the previous width calculation was semantically wrong (particularly problemmatic for markdown links, which is my major motivation for pursuing this PR, see MeanderingProgrammer/render-markdown.nvim#82 for one potential downstream beneficiary).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Samuel Roeca
pappasam left a comment (vim/vim#20472)Note: I just set up a local manual test with https://github.com/preservim/vim-markdown and there are some weird bugs / uncovered cases. So far, we cover X+ conceal, but not Markdown-style conceal of punctuation at line start combined with number, linebreak, breakindent, and showbreak. Will spend a bit of time on that
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.