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

fix: Regression setcellwidths with GUI #14600

Closed

Conversation

h-east
Copy link
Contributor

@h-east h-east commented Apr 20, 2024

A regression has occurred since version 9.1.0344.
(ID-Call as patch authors: @mikoto2000 @zeertzjq)

Related: #14539 #14540

Step to repro.

  1. Start GUI Vim with few parameters.
$ vim -g -Nu NONE +'call setcellwidths([[0x2502, 0x2502, 2]])' +'call setline(1, ["abc\u2502def"])'
  1. Behavior of GUI Vim 9.1.0344
    There is an extra space before d.
    gui_ng_1

    After type fd, The cursor will move to the extra space and d will appear.
    gui_ng

Expected behavior

  1. Behavior of GUI Vim before version 9.1.0343 or with this PR patch applied.
    There is no extra space before d.
    gui_ok_1

    After type fd.
    gui_ok

@chrisbra
Copy link
Member

thanks. Can you please turn that into a regression test?

@h-east
Copy link
Contributor Author

h-east commented Apr 20, 2024

@chrisbra This problem only occurs in the GUI, testing using VerifyScreenDump() (term_dumpdiff()) will not show the difference before and after the fix.
Is there any better way?

@chrisbra
Copy link
Member

no sure, okay let me include it then.

@yegappan
Copy link
Member

@chrisbra This problem only occurs in the GUI, testing using VerifyScreenDump() (term_dumpdiff()) will not show the difference before and after the fix. Is there any better way?

Sometime ago I added the test_gui_event() function to test GUI specific events. We should
add a similar function to test GUI specific rendering of characters.

@chrisbra chrisbra closed this in 8927c9b Apr 20, 2024
@h-east h-east deleted the fix-regression-setcellwidths-with-gui branch April 20, 2024 16:18
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.

None yet

3 participants