Fix terminal line wrapping detection in the TUI. #8915

rickyz · 2018-08-26T00:52:47Z

Fix #7369 by addressing the TODO to implement ui_add_linewrap.

bfredl · 2018-08-26T07:20:51Z

Nice work! I wonder though, if the "hint" couldn't be provided as a boolean flag in the raw_line event (for the line before the wrap), which would make it possible for the TUI to implement linewrap with no extra output in many cases (when the last column is drawn by the raw_line event anyway).

Removing LineWraps makes sense if we don't want to reimplement vim's "modeless" selection (so users will always have to use shift+leftmouse to copy the screen in press-return prompts).

rickyz · 2018-08-27T00:33:26Z

Good point re adding a param to raw_lines. Here's a version which does that.

I'm not super familiar with modeless selection, so I'll defer to your opinion on whether it's worth keeping LineWraps around - what do you think?

rickyz · 2018-08-27T00:34:44Z

src/nvim/screen.c

+        && lcs_eol_one != -1       // Haven't printed the lcs_eol character.
+        && row != endrow - 1       // Not the last line being displayed.
+        && wp->w_width == Columns  // Window spans the width of the screen.
+        && !wp->w_p_rl;            // Not right-to-left.


(Apologies for the repost, not super familiar with github's review processes.)

I've removed the restriction on doing this for lines ending in multibyte characters. As far as I can tell, this restriction dates back to 2000, when it was added to work around a bug affecting kterm (https://marc.info/?l=vim-dev&m=96549912530900&w=2). Since the TUI operates in terms of cells rather than individual bytes, this should not do anything funny for multibyte characters.

rickyz · 2018-08-27T01:00:56Z

Oh, also forgot to mention - the reason this always requires output is: The output for the last cell in the row must immediately precede output of the first cell in the next row (with no terminal control sequences in between)

In particular, adding a cursor_goto call in between the two print_cells breaks the hinting to the terminal.

Because of this, two adjacent calls to tui_raw_line will not trigger line wrapping (due to the cursor_goto call in the second call breaking the line).

If the tui_raw_line calls are guaranteed to occur in sequence, it may be possible to save some output by omitting the first cursor_goto call for continuation lines, but this a little complex, since it relies on assumptions about the ordering of calls to tui_raw_line. This would also need to handle the case where the first column of a line continuation is not dirty.

EDIT: Upon further experimentation, for an 80x24 terminal, the coordinates (row=0, col=80) and (row=1, col=0) appear to be treated a little differently (e.g. the behavior of a carriage return, as used in the implementation of cursor_goto, seems to differ). Based on this, I don't see a clean way to avoid the extra output.

bfredl · 2018-08-27T08:45:52Z

This would also need to handle the case where the first column of a line continuation is not dirty.

I was thinking of doing the same thing here as vim, force invalidate the first cell in the next line by setting the ScreenAttrs value to -1. This will avoid printing the continuation char twice as this branch does sometimes (if the char was going to be drawn anyway).

Because of this, two adjacent calls to tui_raw_line will not trigger line wrapping (due to the cursor_goto call in the second call breaking the line).

cursor_goto() shouldn't output anything if the previous call made sure to already put the cursor in the right position. I e the first tui_raw_line should make sure to change (0, 80) to (1, 0) internally when the wrap flag is sent, by calling final_column_wrap() (which should be safe as nvim core promises that raw_line with wrap will be followed by another raw_line before anything else)

rickyz · 2018-08-27T10:06:16Z

Ah, looks like I was mistaken about the behavior of carriage return - it looks like carriage return does the right thing after a line wrap (go back to the left-most margin of the current physical line).

Here's a version that eliminates the extra output using the method you suggested - I had actually tried calling final_column_wrap() earlier today, but I was missing the bit to invalidate the first col of the next row.

Thanks!

bfredl · 2018-08-27T16:13:16Z

src/nvim/screen.c

+        && row != endrow - 1       // Not the last line being displayed.
+        && wp->w_width == Columns  // Window spans the width of the screen.
+        && !wp->w_p_rl;            // Not right-to-left.
+      if (wrap) {


I would place this after the screen_line call to make it immediately clear it will affect a following screen_line call, and not this one.

bfredl · 2018-08-27T16:17:39Z

Very nice! If you don't mind, let's keep LineWraps for now, if only because #8455 will refactor this data structure, and a LineWraps removal commit made after that will be easier to revert, if we ever decide to reimplement modeless selection again.

rickyz · 2018-08-27T16:31:31Z

Sure thing, re-added LineWraps and moved the ScreenAttrs update down.

Previously, when neovim would wrap a line across multiple lines, terminal emulators could not detect that the lines represent a single wrapped line as opposed to several separate lines. As a result, many terminals' selection/copying functionality would treat a wrapped line as several newline-delimited lines. Fix this by reenabling a "special trick" from Vim. When a line is wrapped, write the last character of that line followed by the first character of the next line to the terminal. This hints to the terminal that the next line is a continuation of the current line. Extends the raw_line event with a "wrap" parameter which controls when to do wrap hinting.

justinmk · 2018-09-09T10:34:15Z

@bfredl any objection to merging this?

bfredl · 2018-09-09T10:43:31Z

Sorry, forgot about this. Looks good to merge.

Remote UI support (and screen tests) would be nice, but I can do it in a follow up PR.

Besides the "visible" improvements, this release features numerous internal improvements to the UI/screen code and test infrastructure. Numerous patches were merged from Vim, which are not mentioned below. FEATURES: 07ad5d7 clipboard: Support custom VimL functions neovim#9304 725da1f neovim#9401 win/TUI: Improve terminal/console support 7a8dadb neovim#9077 startup: Use $XDG_CONFIG_DIRS/nvim/sysinit.vim if it exists feec926 neovim#9299 support <cmd> mapping in more places 0653ed6 neovim#9028 diff/highlight: Show underline for low-priority CursorLine bddcbbb signs: Add "numhl" argument neovim#9113 05f9c7c clipboard: support Wayland (neovim#9230) 14ae394 neovim#9052 TUI: add support for undercurl and underline color 4fa3492 neovim#9023 man.vim: soft (dynamic) wrap neovim#9023 API: 8b39e4e neovim#6920 API: implement object namespaces b1aaa0a API: Implement nvim_win_set_buf() neovim#9100 8de87c7 neovim#8180 API: virtual text annotations (nvim_buf_set_virtual_text) 2b9fc9a neovim#8660 API: add nvim_buf_is_loaded() API: buf_get_lines, buf_line_count handle unloaded buffers 88f77c2 API: nvim_buf_get_offset_for_line 94841e5 API/UI: neovim#8221 ext_newgrid, ext_hlstate (use line-based rather than char-based updates) UI b5cfac0 neovim#8806 TUI: use BCE again more often, (smoother resizes/scrolling) 77b5e9a neovim#9315 screen: add missing status redraw when redraw_later(CLEAR) was used 5f15788 TUI: clip invalid regions on resize (neovim#8779), fixes neovim#8774 c936ae0 neovim#9193 TUI: improvements for scrolling and clearing f204274 neovim#9143 UI: disable clearing almost everywhere f4b2b66 neovim#9079 TUI: always use safe cursor movement after resize d36afaf neovim#9211 ui_options: also send when starting or from OptionSet 67f80d4 TUI: Avoid reset_cursor_color in old VTE neovim#9191 e55ebae neovim#9021 don't erase screen on `:hi Normal` during startup c5790d9 neovim#8915 TUI: Hint wrapped lines to terminals. FIXES: 231de72 RPC: turn errors from async calls into notifications 907ad92 TUI: Restore terminal title via "title stacking" (neovim#9407) cb76a8a genappimage: Unset $ARGV0 at invocation neovim#9376 b48efd9 neovim#9347 TUI: FreeBSD: Improve support for BSD vt console c16529a TUI: Konsole 18.07.70 supports DECSCUSR (neovim#9364) aec096f os/lang: use the correct LC_NUMERIC also for OS X 5fee0be provider: improve error message (neovim#9344) 3c42d7a TUI: alacritty supports set_cursor_color neovim#9353 7bff9a5 TUI: Alacritty supports DECSCUSR (neovim#9048) 57acfce macOS: infer primary language if $LANG is empty neovim#9345 bc132ae runtime/syntax: Fix highlighting of augroup contents (neovim#9328) 715fdfe neovim#9297 VimL/confirm(): Show dialog even if :silent 799d9c3 clipboard: Prefer xclip (neovim#9302) 6dae777 provider/nodejs: fix npm,yarn detection 16bc1e9 neovim#9218 channel: avoid buffering output when only terminal and no callbacks are active 72fecad neovim#8804 Fix crash in lang_init() on macOS if lang_region = NULL d581398 ruby: detect rbenv shims for other versions (neovim#8733) e568ac7 neovim#9123 third-party/unibilium: Fix parsing of extended capability entries c4c74c3 jobstart(): Fix hang on non-executable cwd neovim#9204 1cf50cb provider/nodejs: Simultaneously query npm and yarn neovim#9054 6c496db undo: Fix infinite loop if undo_read_byte returns EOF neovim#2880 f8f8357 neovim#9034 'swapfile: always show dialog' CHANGES: c236e80 neovim#9024 --embed: wait for UI unless --headless 180b50d neovim#9248 python: 'neovim' module was renamed to 'pynvim' 2000b6a neovim#8589 VimL: Remove legacy aliases "v:errmsg", "v:shell_error", "v:this_session" deb18a0 defaults: background=dark neovim#2894 (neovim#9205) c1187d4 defaults: win: 'shellpipe' for cmd.exe (neovim#8827)

rickyz force-pushed the line_wrapping branch 2 times, most recently from 5938cf2 to df89d12 Compare August 27, 2018 00:33

rickyz commented Aug 27, 2018

View reviewed changes

rickyz force-pushed the line_wrapping branch from df89d12 to e8cce59 Compare August 27, 2018 00:51

rickyz force-pushed the line_wrapping branch from e8cce59 to 115716e Compare August 27, 2018 10:01

bfredl reviewed Aug 27, 2018

View reviewed changes

rickyz force-pushed the line_wrapping branch from 115716e to 15260d8 Compare August 27, 2018 16:29

rickyz force-pushed the line_wrapping branch 2 times, most recently from ad291b9 to 03a55e7 Compare September 1, 2018 19:30

rickyz force-pushed the line_wrapping branch from 03a55e7 to 8fd3725 Compare September 3, 2018 10:28

bfredl merged commit c5790d9 into neovim:master Sep 9, 2018

rickyz deleted the line_wrapping branch September 10, 2018 08:57

bfredl mentioned this pull request Oct 18, 2018

restore 'ttyfast' #1045

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix terminal line wrapping detection in the TUI. #8915

Fix terminal line wrapping detection in the TUI. #8915

Uh oh!

rickyz commented Aug 26, 2018

Uh oh!

bfredl commented Aug 26, 2018

Uh oh!

rickyz commented Aug 27, 2018

Uh oh!

rickyz Aug 27, 2018 •

edited

Loading

Uh oh!

rickyz commented Aug 27, 2018 •

edited

Loading

Uh oh!

bfredl commented Aug 27, 2018

Uh oh!

rickyz commented Aug 27, 2018

Uh oh!

bfredl Aug 27, 2018

Uh oh!

bfredl commented Aug 27, 2018

Uh oh!

rickyz commented Aug 27, 2018

Uh oh!

justinmk commented Sep 9, 2018

Uh oh!

bfredl commented Sep 9, 2018

Uh oh!

Uh oh!

Uh oh!

Fix terminal line wrapping detection in the TUI. #8915

Fix terminal line wrapping detection in the TUI. #8915

Uh oh!

Conversation

rickyz commented Aug 26, 2018

Uh oh!

bfredl commented Aug 26, 2018

Uh oh!

rickyz commented Aug 27, 2018

Uh oh!

rickyz Aug 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rickyz commented Aug 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bfredl commented Aug 27, 2018

Uh oh!

rickyz commented Aug 27, 2018

Uh oh!

bfredl Aug 27, 2018

Choose a reason for hiding this comment

Uh oh!

bfredl commented Aug 27, 2018

Uh oh!

rickyz commented Aug 27, 2018

Uh oh!

justinmk commented Sep 9, 2018

Uh oh!

bfredl commented Sep 9, 2018

Uh oh!

Uh oh!

rickyz Aug 27, 2018 •

edited

Loading

rickyz commented Aug 27, 2018 •

edited

Loading