Skip to content

fix(window): improve cursor screen position calculation for multibyte chars #1853

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

Merged
merged 4 commits into from
Jun 13, 2025

Conversation

petobens
Copy link
Contributor

@petobens petobens commented Jun 4, 2025

Update get_cursor_screen_position to use display width for accurate column indexing and add error handling for screenpos failures.
This ensures correct positioning with wide or multibyte characters and prevents runtime errors.

Fixes #1851

… characters

Update get_cursor_screen_position to use display width for accurate column indexing and add error handling for screenpos failures. This ensures correct positioning with wide or multibyte characters and prevents runtime errors.
@soifou

This comment was marked as outdated.

@petobens

This comment was marked as outdated.

@petobens

This comment was marked as outdated.

@soifou

This comment was marked as outdated.

@petobens

This comment was marked as outdated.

@petobens

This comment was marked as outdated.

@soifou

This comment was marked as outdated.

@Saghen
Copy link
Owner

Saghen commented Jun 9, 2025

:h screenpos says that it should take a byte-index for the column so I don't think we should be using the string display width here. In #1874, eokoshi mentions that the window id from nvim.api.nvim_win_get_number(0) appears to be wrong, which would explain why it's complaining about the line not existing.

@petobens
Copy link
Contributor Author

Indeed the fix suggested in #1874 seems to work and is cleaner.

@pedro757
Copy link

I'm using this and it's fine, doesn't break anything! waiting for the merge!

@Saghen
Copy link
Owner

Saghen commented Jun 11, 2025

I'd like to figure out why we need to use the win_getid rather than blindly accepting. Afaict, nvim.api.nvim_win_get_number(0) and vim.fn.win_getid() should return the same window id.

@soifou
Copy link
Collaborator

soifou commented Jun 11, 2025

I'd like to figure out why we need to use the win_getid rather than blindly accepting

Because this bug has been introduced by upstream commit eeacd7bd71 and from what I can tell, this change made the parent window logic stricter. So we cannot rely on nvim.api.nvim_win_get_number anymore?

We can use also simply 0:

local pos = vim.fn.screenpos(0, cursor_line, cursor_column)

@Saghen Saghen merged commit fe44a7d into Saghen:main Jun 13, 2025
4 checks passed
@Saghen
Copy link
Owner

Saghen commented Jun 13, 2025

Thank you all!

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.

error in winresize autocmd call to screenpos
4 participants