Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buffer_updates: set deleted_bytes correctly when hitting ~ #12646

Merged
merged 2 commits into from
Jul 21, 2020

Conversation

vigoux
Copy link
Member

@vigoux vigoux commented Jul 16, 2020

This PR fixes #12645
The problem was the deleted_bytes of buf_t not getting updated in the pbytes function, causing every feature relying on this to send wrong buffer updates (and by the same occasion making the treesitter tree out of sync).

@vigoux vigoux force-pushed the ts-fix-tilde branch 2 times, most recently from a6fdf8f to 1a80857 Compare July 20, 2020 17:15
@vigoux vigoux changed the title treesitter: recover from strange buffer updates buffer_updates: set deleted_bytes correctly when hitting ~ Jul 20, 2020
src/nvim/ops.c Outdated

line[lp.col] = (char_u)c;

ml_add_deleted_len(line, old_len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to go for a purer style ml_add_deleted_len_buf

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also you could pass old_len = -1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the true in ml_get_buf(curbuf, lp.lnum, true) sets the line dirty so I suppose it should update statistics at some point. Maybe it happens too late ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some digging, it seems (I may be wrong here) that the buf->deleted_bytes is not related to the fact that the line is dirty.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus the name deleted_bytes is actually misleading, as this represent the old byte size for buffer updates.
I think that this old_byte_size should be computed just before sending the buffer update, in the extmarks code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since old_byte_size has to be lines anyway, shouldn't it be updated here https://github.com/neovim/neovim/blob/master/src/nvim/memline.c#L1879 instead ?
There are some other calls of ml_get_buf(_, _, true) that won't be taken care of otherwise (I could find examples throughout the code. This can be revisited in the bytetrack PR (maybe add it as a comment so that we don't forget).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's far better

Test this using treesitter highlighting, which is based on this
old_byte_size.
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.

Invalid old_byte_size parameter on buffer updates for ~
2 participants