[vim/vim] Fix several bugs in the text property (PR #19869)
h_east
Related: #19685 (Since the original PR is no longer being updated due to various reasons, I am taking over the work in this PR.)
Fixes: #19680
Fixes: #19681
Fixes: #12568
Fixes: #19256
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/19869
Commit Summary
- 6fe1675 Fix several bugs in the text property
File Changes
(11 files)
- M src/charset.c (22)
- M src/drawline.c (36)
- M src/ex_cmds.c (32)
- M src/memline.c (75)
- M src/ops.c (41)
- M src/proto/textprop.pro (12)
- M src/structs.h (45)
- M src/testdir/Make_all.mak (2)
- M src/testdir/test_textprop.vim (10)
- A src/testdir/test_textprop2.vim (421)
- M src/textprop.c (1316)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- 6109871 Fix by self-review
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- 4fa1768 Fix CI error
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- d5e4a0b Fix CI error
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- c912d9c Fix expected test value in Test_multiline_substitute_del_lines_drops_virt_text_props() L41
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- 977b593 Fix CI leak error
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 2 commits.
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 4 commits.
- aedadc0 Fix expected test value in Test_multiline_substitute_del_lines_drops_virt_text_props() L41
- 69934d8 Fix flaky Test_props_with_text_after_nowrap
- d29b1f8 Fix by self-review
- 387b552 Fix CI ASAN error
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 2 commits.
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 2 commits.
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 2 commits.
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#19869)Specification Differences
| Item | this-pr | pr-19685 (#19685) |
|---|---|---|
| Virtual text placement types | Existing RIGHT / ABOVE / BELOW only (same as master) | AFTER, INLINE added |
| Placement flag scheme | Independent bits (same as master) | Multi-value encoding using bit4-6 (TP_FLAG_VTEXT_BITS = 0x070) |
TP_FLAG_ALIGN_BELOW value |
0x040 (same as master) |
Changed to 0x030 (not compatible) |
Inline vtext and start_incl |
N/A | Doc added: controls insertion position |
prop_list() return order |
Unchanged (by starting column and priority) | Changed to "by starting column" |
| Negative ID restriction (E1293/E1339) | Unchanged | Removed from documentation |
PROP_IS_* macros |
None | PROP_IS_VTEXT, PROP_IS_FLOATING, PROP_IS_INLINE, etc. |
Behavioral Difference
When a multi-line substitute joins lines 1-3 (1,3 s/e.1\nLine..2\nL/e.1 L/), floating virtual text properties on deleted lines are handled differently:
- this-pr: vtext props on deleted line 2 are moved to line 1 and preserved.
prop_list(1)returns 4. - pr-19685: vtext props on deleted line 2 are dropped.
prop_list(1)returns 3.
Implementation: Common
- Virtual text stored directly in memline (
b_textprop_textremoved) prop_countheader (uint16_t) added to memline format:[text][NUL][prop_count][textprop_T...][vtext...]unpacked_memline_Tintroduced (LOADED / DETACHED / CLOSED states)tp_text_offset/tp_textunion added totextprop_TTP_FLAG_DELETED (0x200)added
Implementation: Differences
textprop_T
| Item | this-pr | pr-19685 |
|---|---|---|
| Union naming | Named u (u.tp_text, u.tp_text_offset) |
Anonymous union (direct tp_text, tp_text_offset) |
| Union discriminator | Explicit TP_FLAG_VTEXT_PTR (0x400) flag |
TP_FLAG_VTEXT_BITS mask != 0 means vtext |
tp_len for vtext |
STRLEN(vtext) (excluding NUL) |
vtext size (including NUL) |
unpacked_memline_T
| Item | this-pr | pr-19685 |
|---|---|---|
| Member count | 9 | 12 (next/prev/adjust_flags added) |
| Adjacent line management | None (existing adjust_text_props_for_delete adapted) |
next/prev linked list for batch processing |
prop_size/prop_count type |
int |
uint16_t |
um_* API
| Item | this-pr | pr-19685 |
|---|---|---|
| Public function count | 10 | 16 |
um_pack |
static (used internally via um_store_changes) |
public |
um_store_changes |
Yes (auto-save on um_goto_line/um_close) |
None (caller must explicitly um_pack + ml_replace_len) |
um_open_at_detached |
None | Yes |
um_extract_props |
None | Yes |
um_sort_props |
None | Yes |
Other
| Item | this-pr | pr-19685 |
|---|---|---|
| Internal types | adjustres_T only |
criteria_T, prop_remove_info_T, vtext_T, prop_mod_T added |
vtext_T (ref_count) |
None | Defined but unused (future use or leftover) |
| memline.c changes | Minimal (prop_count header adaptation) | Rewritten using um_* API (e.g. add_text_props_for_append) |
| undo.c | No changes | has_prop_w_flags removed, ML_PROPS_INCLUDED/ML_COPY_LINE constants |
| Test infrastructure | Vim script tests only | Python test helper (textprop.py, textprop_support.vim) added |
| Change size | +1,683 / -368 (16 files) | +4,300 / -1,433 (38 files) |
Design Philosophy
- this-pr: Conservative approach. Preserves existing code as much as possible, focusing on memline format change and
unpacked_memline_Tintroduction.um_store_changesauto-saves to reduce caller burden. - pr-19685: Large-scale refactoring of text property subsystem. Adjacent line linked list, INLINE/AFTER features, public
um_pack, designed for future extensibility.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 7 commits.
- 8126316 Fix several bugs in the text property
- 3ef5aaf Fix expected test value in Test_multiline_substitute_del_lines_drops_virt_text_props() L41
- 4a578ac Fix flaky Test_props_with_text_after_nowrap
- 6dff5fb Fix by self-review
- 5935b42 Fix CI ASAN error
- 54cda49 Use bool/true/false
- 8874029 Remove unused b_textprop_text field
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.