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

vim-patch:8.2.{1535,1537,3545}: setcellwidths() #19674

Merged
merged 6 commits into from
Aug 8, 2022

Conversation

zeertzjq
Copy link
Member

@zeertzjq zeertzjq commented Aug 8, 2022

vim-patch:8.2.1535: it is not possible to specify cell widths of characters

Problem: It is not possible to specify cell widths of characters.
Solution: Add setcellwidths().
vim/vim@08aac3c

Co-Authored-By: delphinus me@delphinus.dev

vim-patch:8.2.1537: memory acccess error when using setcellwidths()

Problem: Memory acccess error when using setcellwidths().
Solution: Use array and pointers correctly.
vim/vim@b06a6d5

vim-patch:8.2.3545: setcellwidths() may make 'listchars' or 'fillchars' invalid

Problem: setcellwidths() may make 'listchars' or 'fillchars' invalid.
Solution: Check the value and give an error. (closes vim/vim#9024)
vim/vim@94358a1

Cherry-pick f_setcellwidths() change from patch 9.0.0036.
Cherry-pick 'ambiwidth' docs update from runtime update 079ba76ae7a7.

@github-actions github-actions bot added the vim-patch See https://neovim.io/doc/user/dev_vimpatch.html label Aug 8, 2022
@zeertzjq zeertzjq changed the title vim-patch:8.2.1535,8.2.1537,8.2.3545 vim-patch:8.2.{1535,1537,3545}: setcellwidth() Aug 8, 2022
@github-actions github-actions bot requested a review from seandewar August 8, 2022 04:37
@zeertzjq zeertzjq changed the title vim-patch:8.2.{1535,1537,3545}: setcellwidth() vim-patch:8.2.{1535,1537,3545}: setcellwidths() Aug 8, 2022
@delphinus
Copy link
Contributor

Thx for rebasing. Now Neovim's view is broken which Vim can draw validly. I confirmed this on this PR & my one (#13883).

# ℃ <- 0x2103
$ cat /tmp/aaa.txt
''

on Neovim

$ nvim -N -u NONE -i NONE --noplugin +'call setcellwidths([[0x2103, 0x2103, 2]])' /tmp/aaa.txt

スクリーンショット 0004-08-08 13 57 42

$ vim +'call setcellwidths([[0x2103, 0x2103, 2]])' /tmp/aaa.txt

スクリーンショット 0004-08-08 13 58 41

On Neovim, it puts ' over the character .Vim draws separatedly without fail.

@zeertzjq
Copy link
Member Author

zeertzjq commented Aug 8, 2022

I think this is likely because Nvim's TUI is line-based, while Vim's TUI is char-based. Does iTerm2 also behave like this if you cat /tmp/aaa.txt?

@delphinus
Copy link
Contributor

Yes. iTerm2 behaves similarly as U+2301 has 1 width because this option below is disabled in my box.

  • Preferences → Profiles → Text → Ambiguous characters are double-width (default: OFF)
OFF ON
スクリーンショット 0004-08-08 18 17 48 スクリーンショット 0004-08-08 18 16 49

I expect setcellwidths() should work as overwriting this...

I think this is likely because Nvim's TUI is line-based, while Vim's TUI is char-based.

We should adjust logic for win_line() in screen.c ?

@zeertzjq
Copy link
Member Author

zeertzjq commented Aug 8, 2022

win_line() does not need change. It is likely that cursor_goto() and print_cell() in tui/tui.c may need change. Sorry, my previous comment does not make sense.

@zeertzjq
Copy link
Member Author

zeertzjq commented Aug 8, 2022

If performance is not taken into consideration, you may fix this problem by making this small change:

diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
index e2289eb9c..78281470d 100644
--- a/src/nvim/tui/tui.c
+++ b/src/nvim/tui/tui.c
@@ -788,6 +788,7 @@ static void cursor_goto(UI *ui, int row, int col)
 {
   TUIData *data = ui->data;
   UGrid *grid = &data->grid;
+  goto safe_move;
   if (row == grid->row && col == grid->col) {
     return;
   }

This change is not desirable as it disables a bunch of performance improvements.

zeertzjq and others added 6 commits August 8, 2022 20:03
This just avoids including mbyte.h in eval/typval.h, so that mbyte.h can
include eval/typval.h in Vim patch 8.2.1535.
…acters

Problem:    It is not possible to specify cell widths of characters.
Solution:   Add setcellwidths().
vim/vim@08aac3c

Co-Authored-By: delphinus <me@delphinus.dev>
Problem:    Memory acccess error when using setcellwidths().
Solution:   Use array and pointers correctly.
vim/vim@b06a6d5
…s' invalid

Problem:    setcellwidths() may make 'listchars' or 'fillchars' invalid.
Solution:   Check the value and give an error. (closes vim/vim#9024)
vim/vim@94358a1

Cherry-pick f_setcellwidths() change from patch 9.0.0036.
Cherry-pick 'ambiwidth' docs update from runtime update 079ba76ae7a7.
@zeertzjq zeertzjq merged commit d36c2fa into neovim:master Aug 8, 2022
@zeertzjq zeertzjq deleted the vim-8.2.1535 branch August 8, 2022 13:20
@github-actions github-actions bot removed the request for review from seandewar August 8, 2022 13:20
@zeertzjq
Copy link
Member Author

zeertzjq commented Aug 9, 2022

I have a solution to the problem mentioned above in #19686.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vim-patch See https://neovim.io/doc/user/dev_vimpatch.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants