Cimbali

unread,

Jun 11, 2021, 12:02:55 PM6/11/21

to vim/vim, Subscribed

This PR attempts to fix #2865 by removing newlines marked as « continuation » line feeds from the terminal output, when pushing these lines to the buffer.

This allows e.g. to open a file with gf when the file name is long and split across several lines. Also nice visually because the

The good:

it compiles
it doesn’t segfault
it works for me: long lines are now a single line in the buffer (whether they are added when scrollback is enabled or paused etc.), and the buffer wrapping mechanism takes care of the displaying them nicely. See screenshots below.

The bad:

absolutely not sure this is how it should be implemented. In particular:
- the changes inside add_scrollback_line_to_buffer(): Is ml_replace the correct approach ?
- Is it OK to expose vterm’s line infos via vterm_state_get_line_info ?
- Is it OK to modify the sb_pushline callback ? It seems to be a minimal change and I’ve checked other occurrences of this callback.
the Windows non-utf8 wide char case in add_scrollback_line_to_buffer() is not done yet
no test cases
might have messed up code-style or code-quality wise, I’m unsure
resizing the terminal window still causes loss of data

Asking for any and all opinions at this stage.

I’ve added the continuation info to the scrollback lines, which may not be 100% necessary (there could be a work-around) at this time − however I believe this will be useful to « reflow » lines, that is move the continuation line breaks on window resizes. See discussion in #2865.

Terminal mode: long line causing « continuation » line feeds (same for both vim version):

Switching to normal mode (current vim master), long output line is artifically split, causes problems to select as 1 word and messy display:

Switching to normal mode (vim built from this PR), long output line appears as a single line (see line numbers), line split is moved due to gutter size, single word split across displayed lines is really a single word:

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/8365

Commit Summary

Unwrap lines as they are sent to buffer
Correctly unsplit lines in callback for lines pushed offscreen
Add continuation info to scrollback lien entries
Handle unwrapping terminal-to-buffer lines in postponed scrollback

File Changes

Patch Links:

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Cimbali

unread,

Jun 11, 2021, 12:31:42 PM6/11/21

to vim/vim, Subscribed

Appveyor build fails on this line ? Not sure why

https://github.com/vim/vim/blob/591660b47efd8398333b35ec9c61dc1b3a065bf1/src/libvterm/src/screen.c#L217

James McCoy

unread,

Jun 11, 2021, 6:25:38 PM6/11/21

to vim/vim, Subscribed

Cc @leonerd

Cimbali

unread,

Jun 12, 2021, 4:59:53 AM6/12/21

to vim/vim, Push

@Cimbali pushed 4 commits.

d53fc93 Unwrap lines as they are sent to buffer
d2d6427 Correctly unsplit lines in callback for lines pushed offscreen
ea5ca91 Add continuation info to scrollback lien entries
32f0c86 Handle unwrapping terminal-to-buffer lines in postponed scrollback

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or unsubscribe.

Cimbali

unread,

Jun 13, 2021, 4:41:18 AM6/13/21

to vim/vim, Push

@Cimbali pushed 4 commits.

a7f89b7 Unwrap lines as they are sent to buffer
1221964 Correctly unsplit lines in callback for lines pushed offscreen
ac67be5 Add continuation info to scrollback lien entries
bb0cbd6 Handle unwrapping terminal-to-buffer lines in postponed scrollback

—
You are receiving this because you are subscribed to this thread.

K.Takata

unread,

Jun 14, 2021, 1:40:46 AM6/14/21

to vim/vim, Subscribed

Appveyor build fails on this line ? Not sure why

Because MSVC 10 doesn't support C99 style variable declaration.
All the variables should be declared at the top of the block.

Cimbali

unread,

Jun 14, 2021, 2:36:45 AM6/14/21

to vim/vim, Push

@Cimbali pushed 1 commit.

d9115c2 Declare at top of block

—
You are receiving this because you are subscribed to this thread.

codecov[bot]

unread,

Jun 14, 2021, 5:42:18 AM6/14/21

to vim/vim, Subscribed

Codecov Report

Merging #8365 (ab72d87) into master (3044324) will decrease coverage by 87.26%.
The diff coverage is 2.42%.

❗ Current head ab72d87 differs from pull request most recent head d9115c2. Consider uploading reports for the commit d9115c2 to get more accurate results

@@             Coverage Diff             @@

##           master    #8365       +/-   ##

===========================================

- Coverage   89.77%    2.50%   -87.27%     

===========================================

  Files         149      147        -2     

  Lines      167282   162254     -5028     

===========================================

- Hits       150178     4068   -146110     

- Misses      17104   158186   +141082

Flag	Coverage Δ
huge-clang-none	`?`
huge-gcc-none	`?`
huge-gcc-testgui	`?`
huge-gcc-unittests	`2.50% <2.42%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/cmdexpand.c	`0.00% <0.00%> (-93.75%)`	⬇️
src/eval.c	`0.10% <0.00%> (-96.08%)`	⬇️
src/evalvars.c	`5.31% <0.00%> (-91.03%)`	⬇️
src/libvterm/include/vterm.h	`0.00% <ø> (-37.50%)`	⬇️
src/libvterm/src/screen.c	`0.00% <0.00%> (-60.58%)`	⬇️
src/libvterm/src/state.c	`0.00% <0.00%> (-90.00%)`	⬇️
src/scriptfile.c	`0.94% <0.00%> (-90.59%)`	⬇️
src/spell.c	`3.71% <0.00%> (-85.32%)`	⬇️
src/spellfile.c	`0.76% <0.00%> (-88.17%)`	⬇️
src/spellsuggest.c	`0.78% <0.00%> (-92.88%)`	⬇️
... and 154 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3044324...d9115c2. Read the comment docs.

Cimbali

unread,

Jun 14, 2021, 6:03:41 AM6/14/21

to vim/vim, Push

@Cimbali pushed 3 commits.

a2685cc Correctly unsplit lines in callback for lines pushed offscreen
6033747 Add continuation info to scrollback line entries
cafce6f Handle unwrapping terminal-to-buffer lines in postponed scrollback

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jun 14, 2021, 6:40:22 AM6/14/21

to vim/vim, Push

@Cimbali pushed 3 commits.

8ed2c4b Correctly unsplit lines in callback for lines pushed offscreen
b2362fc Add continuation info to scrollback line entries
8c917d3 Handle unwrapping terminal-to-buffer lines in postponed scrollback

—
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Jun 14, 2021, 6:57:02 AM6/14/21

to vim/vim, Subscribed

Just FYI: I was pinged on this PR but I'm not intending to look into it all that much. vim's forked copy of libvterm has continued to diverge from my upstream copy to the point that it isn't easy to synchronise these things in again. In any case this particular development has happened entirely without my thoughts or input on the matter, and is somewhat different to the still in-progress work I have in a similar direction upstream.

Overall if youfolks want to try to synchronise these things back together again we need to do a much better job of coördinating things; otherwise I suspect you're on your own at this point.

Cimbali

unread,

Jun 14, 2021, 8:04:55 AM6/14/21

to vim/vim, Push

@Cimbali pushed 4 commits.

a1ae906 Unwrap lines as they are sent to buffer
7d013b6 Correctly unsplit lines in callback for lines pushed offscreen
e3880a9 Add continuation info to scrollback line entries
a957733 Handle unwrapping terminal-to-buffer lines in postponed scrollback

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jun 14, 2021, 8:09:44 AM6/14/21

to vim/vim, Subscribed

TBH @leonerd this PR modifies one callback from libvterm, the rest are just modifications to vim. There’s no real functional changes to libvterm: text reflowing is not implemented in libvterm in this PR − only in the vim buffer.

The sb_pushline callback is changed to send some info on the line being pushed (specifically the VTermLineInfo since we need to know whether the line has a continuation marker):

https://github.com/vim/vim/blob/7d013b61ca316fe4accdb4acadd9bb7faef156fb/src/libvterm/include/vterm.h#L492

If anyone suggests a better way I’m happy to go with it:

The number of the row being pushed back by the sb_pushline (row from sb_pushline_from_row(VTermScreen *screen, int row)) does not seem trivial to find out without modifying the callback (at least to my novice eyes). However if someone can suggest how to do that I can leave the callback unchanged.
If it’s better to only send the row number instead of the VTermLineInfo I can do that too: the lineinfo array is accessible through the cbdata in this case.

Bram Moolenaar

unread,

Jun 14, 2021, 9:02:01 AM6/14/21

to vim/vim, Subscribed

@leonerd You can easily subscribe to Vim changes and filter on "libvterm" to see what changes are made in the Vim copy of libvterm code.
Is there a way to subscribe to changes in the root libvterm? Not the copy that neovim has made (I believe it lags behind, I rather follow the original).

Cimbali

unread,

Jun 16, 2021, 6:56:17 AM6/16/21

to vim/vim, Push

@Cimbali pushed 4 commits.

77d6b39 Unwrap lines as they are sent to buffer
d793a58 Correctly unsplit lines in callback for lines pushed offscreen
66789f2 Add continuation info to scrollback line entries
3db2e6a Handle unwrapping terminal-to-buffer lines in postponed scrollback

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jun 18, 2021, 8:43:35 AM6/18/21

to vim/vim, Push

@Cimbali pushed 1 commit.

a2fd198 Handle Windows wide char case

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jun 18, 2021, 11:47:33 AM6/18/21

to vim/vim, Subscribed

I did the missing case for Windows wide-chars, but this approach seems somwhat doomed I’m afraid due to unforeseen interactions with scroll.

When the terminal is scrolled, lineinfos are shifted before calling scrollrect, which in turn calls the sb_pushline callback − this makes it impossible to find out whether lines that were just pushed out of screen are continuations of the previous line.
scrollrect actually seems to require the lineinfo to be scrolled, as shifting after lineinfo after causes « DWL spill over », i.e. the last test of libvterm’s test 67 fails.

The ways this manifests in vim is that lines are incorrectly joined (joins are shift by 1) when the continuation lines are offscreen.

Simon Sobisch

unread,

Nov 26, 2021, 6:15:01 AM11/26/21

to vim/vim, Subscribed

So we either have scrolling broken or the automatic resize?

Especially in TermDebug sessions with GDB often having to output longer content it would be very nice if making the gdb window wider would also show the content instead of multiple hard-broken lines and a lot of spaces.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

Cimbali

unread,

Nov 26, 2021, 9:39:06 AM11/26/21

to vim/vim, Subscribed

At least that’s the issue with this attempt of implementing reflow @GitMensch. I’m sure there could be another way, but likely not without some input from people who have a better understanding of libvterm and/or vim’s src/terminal.c code.

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Nov 27, 2021, 1:56:03 PM11/27/21

to vim/vim, Push

@Cimbali pushed 6 commits.

ff9fd57 Unwrap lines as they are sent to buffer
4ba1ade Correctly unsplit lines in callback for lines pushed offscreen
1e56511 Add continuation info to scrollback line entries
fb7c61c Handle unwrapping terminal-to-buffer lines in postponed scrollback
4be086a Handle Windows wide char case
fb93459 Update lineinfo after scrolling instead of before

—
You are receiving this because you are subscribed to this thread.

View it on GitHub.

Cimbali

unread,

Nov 27, 2021, 2:49:38 PM11/27/21

to vim/vim, Subscribed

After some more debugging I’ve managed to actually make this work as expected. Just to reiterate, this doesn’t handle resizing the terminal when it’s running, just displaying the terminal contents without undesired linebreaks when in normal buffer mode (and all that entails, see discussion above).

I’ve opened a discussion on the libvterm launchpad to see whether this would cause some additional divergence, or if we can keep that as low as possible.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub.

damnskippy

unread,

Nov 28, 2021, 11:55:15 AM11/28/21

to vim/vim, Subscribed

Sounds like this will address #5769 as well if I'm not mistaken.
For me personally it's the wraps when using termdebug that's the hardest to deal with that @GitMensch is also alluding to.

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Dec 6, 2021, 8:01:20 PM12/6/21

to vim/vim, Subscribed

I haven’t heard back from @leonerd yet, but I’ve been able to have a look at his work on reflowing in libvterm and the changes proposed there seem perfectly complementary to those in this PR. In the mean time, I’ve setup some mirrors so you can visualize the libvterm reflow changes etc.

—
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Dec 8, 2021, 1:20:31 PM12/8/21

to vim/vim, Subscribed

Hi @Cimbali what are you waiting for from me?

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Dec 8, 2021, 7:23:37 PM12/8/21

to vim/vim, Subscribed

Just your input on the changes affecting libvterm really (of which I also provided a rebased version on the upstream codebase on launchpad).

There’s a callback change, so it’s a pretty significant change. However I believe it fits in well with your work in progress on reflowing. Roughly, it allows history to be reflowed, so in this instance the vim buffer. It will also allow to pull lines with proper length from history with sb_popline after resize (or during resize if adding more rows to the window).

If you think these changes are good and you’re likely to integrate them into your reflow work, then I suppose it’s low risk to merge them here. If you’d prefer another way of doing, I can look into it. And if you’d think you’d rather not do these changes in ilbvterm at all, it’s up to @brammool to arbitrate the risk of diverging codebases further versus the added functionality.

—
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Dec 9, 2021, 8:28:15 AM12/9/21

to vim/vim, Subscribed

I've added comment on the launchpad issue; copied here:

Overall this looks quite a lot of code change to have without any sort of unit tests. I'd like to see some tests.

Moreover, I'd like to have seen the tests first as a way to discuss what actual behaviour is required. Once we agree that's useful I can likely go about implementing it.

But please start first with the tests. Always.

—
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jan 22, 2022, 7:02:17 PM1/22/22

to vim/vim, Subscribed

Hi @leonerd, just a ping to notify you that I replied on launchpad last month and added a test. It’s also in this PR.

The changeset is really small, just adding a value to a callback. Please let us know if you plan on reviewing it.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Sergey Vlasov

unread,

Feb 8, 2022, 11:35:12 AM2/8/22

to vim/vim, Subscribed

@Cimbali I tried the patch and it mostly works great. However there is a problem with very long lines.

Try seq 1000 | paste -sd+ several times, go to normal mode and scroll the buffer. You will notice that at some point the line breaks disappear and colors from the shell prompt (if your prompt has colors) shift to other lines or completely disappear.

—
Reply to this email directly, view it on GitHub.

Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Feb 8, 2022, 5:56:38 PM2/8/22

to vim/vim, Subscribed

Thanks for testing @noscript, I'll look into it. Do you maybe have a screenshot?

—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Sergey Vlasov

unread,

Feb 9, 2022, 10:26:26 AM2/9/22

to vim/vim, Subscribed

vim -g --clean +'set lines=30 columns=22' +'term bash --noprofile --norc' +'only'

then

seq 100 | paste -sd+
tput setab 1; echo "this is red text"; tput sgr0
seq 100 | paste -sd+

then go to normal mode

—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.

Simon Sobisch

unread,

Apr 25, 2022, 2:41:48 PM4/25/22

to vim/vim, Subscribed

@Cimbali Any updates? Finishing this would be so helpful for debugging...

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Apr 26, 2022, 4:32:46 AM4/26/22

to vim/vim, Subscribed

I can reproduce noscript’s issue but did not have time to delve into it much further I’m afraid.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Aug 8, 2022, 6:34:16 AM8/8/22

to vim/vim, Push

@Cimbali pushed 2 commits.

12c8d52 Add VTermLineInfo* to libvterm sb_pushline callback
9689a92 Unwrap terminal lines in buffer

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Aug 8, 2022, 7:01:00 AM8/8/22

to vim/vim, Subscribed

I fixed the last line unwrapping too eagerly. The next issue is the assumption that the number of buffer lines and scrollback lines are the same doesn’t hold any longer. This messes up cleanup_scrollback() from what I can see, as to few lines are popped from the buffer, and thus switching from normal to terminal repeatedly adds the same parts of some lines to the buffer.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Aug 25, 2022, 12:11:00 PM8/25/22

to vim/vim, Push

@Cimbali pushed 6 commits.

de6d478 Add VTermLineInfo* to libvterm sb_pushline callback
78dfc1a Unwrap terminal lines in buffer
7d45592 Only remove from buffer the lines that were added for snapshot
d7b91b4 Update limit_scrollback to take line continuations into account
45d21b0 Parallel tracking of buffer lines and scrollback lines
a4c1e72 Ensure the first line is not continuation on clear cmd

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 1, 2022, 8:05:58 AM9/1/22

to vim/vim, Subscribed

Still slightly a WIP: I’ve managed to work out all the kinks I could find and wrapping works for terminal usage, including properly colouring scrolled text etc − only terminal dump read/write (whose tests fail) still needs some love.

There is something I am slightly unhappy about in my implementation: whenever information from sb_line_T is needed in the buffer (typically cell attributes), the tl_scrollback garray needs to be walked to find the corresponding line. For on-screen line it’s not much work (though it happens for every cell being redrawn), since the search can happen from the top of the screen (we keep track of this to un-scroll lines when returning to terminal mode). However for a buffer being scrolled upwards, the lines coming into view need to be found and this search happens from the beginning of the buffer.

Basically: it works, but I’m afraid there could be lagging when scrolling up large terminal buffers in normal mode.

There’s 2 potential solutions I can think of that could be added:

Store in each sb_line_T the corresponding line number in the buffer. Seems the simplest but probably requires a lot of work whenever cleanup_scrollback() is called to remove the (10%?) top lines of a buffer.
Cache (e.g. in term_S?) the latest buffer line -> scrollback line lookup result and start search from there (if it’s closer than the top of the buffer). That would make repeated calls for lookups on the same or neighbouring lines very efficient.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 1, 2022, 12:10:11 PM9/1/22

to vim/vim, Push

@Cimbali pushed 3 commits.

f74363a Parallel tracking of buffer lines and scrollback lines
b2d0999 Call scrollbackline_pos_in_buf() with absolute row number
a6fc03b Ensure the first line is not continuation on clear cmd

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 5, 2022, 6:57:08 AM9/5/22

to vim/vim, Push

@Cimbali pushed 8 commits.

4a0b838 Add a test for terminal unwrapping
72240ab Add VTermLineInfo* to libvterm sb_pushline callback
c76d317 Unwrap terminal lines in buffer
c04b5be Only un-scroll from buffer the lines that were added for snapshot
d4d9071 Update limit_scrollback to take line continuations into account
98f5893 Parallel tracking of buffer lines and scrollback lines
2ac37be Call scrollbackline_pos_in_buf() with absolute row number
6d509e7 Ensure the first line is not continuation on clear cmd

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 6, 2022, 1:00:35 PM9/6/22

to vim/vim, Push

@Cimbali pushed 1 commit.

a3e405b Do not lookup bufline if no scrollback

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 9, 2022, 7:20:50 AM9/9/22

to vim/vim, Push

@Cimbali pushed 9 commits.

b91f00b Add a test for terminal unwrapping
7ff816b Add VTermLineInfo* to libvterm sb_pushline callback
87ea613 Unwrap terminal lines in buffer
5df293c Only un-scroll from buffer the lines that were added for snapshot
0554d96 Update limit_scrollback to take line continuations into account
132e325 Parallel tracking of buffer lines and scrollback lines
edcac0e Call scrollbackline_pos_in_buf() with absolute row number
5804680 Ensure the first line is not continuation on clear cmd
2d40848 Do not lookup bufline if no scrollback

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Simon Sobisch

unread,

Sep 9, 2022, 9:12:08 AM9/9/22

to vim/vim, Subscribed

The initial "bad" points are fixed and the reported wrapping issues seem to be fixed (according to the commits), correct?

@Cimbali: it's there anything you'd like to tackle before this is (hopefully) merged?

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 14, 2022, 5:07:11 AM9/14/22

to vim/vim, Subscribed

Thanks for checking in @GitMensch! It should be 100% done, functionally. I’d be happy to get feedback on it.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 14, 2022, 6:54:42 AM9/14/22

to vim/vim, Push

@Cimbali pushed 7 commits.

d434119 Only un-scroll from buffer the lines that were added for snapshot
7e5efb6 Update limit_scrollback to take line continuations into account
11b706e Parallel tracking of buffer lines and scrollback lines
a5ef035 Call scrollbackline_pos_in_buf() with absolute row number
2dcf6cc Ensure the first line is not continuation on clear cmd
f2c3435 Do not lookup bufline if no scrollback
61526bf Tweaks to make bytes unsigned

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 14, 2022, 9:13:04 AM9/14/22

to vim/vim, Push

@Cimbali pushed 1 commit.

a35c407 Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 14, 2022, 9:31:01 AM9/14/22

to vim/vim, Push

@Cimbali pushed 1 commit.

56f16ad Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 14, 2022, 1:38:49 PM9/14/22

to vim/vim, Subscribed

Tried fixing the test but I’m not really sure why it doesn’t work 🤔 It does locally.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Nov 5, 2022, 9:13:39 AM11/5/22

to vim/vim, Push

@Cimbali pushed 11 commits.

59f260c Add a test for terminal unwrapping
d2296a9 Add VTermLineInfo* to libvterm sb_pushline callback
770f323 Unwrap terminal lines in buffer
6de4d8b Only un-scroll from buffer the lines that were added for snapshot
46dc1ac Update limit_scrollback to take line continuations into account
1d21f5c Parallel tracking of buffer lines and scrollback lines
559b7cb Call scrollbackline_pos_in_buf() with absolute row number
3f059c2 Ensure the first line is not continuation on clear cmd
4f7e66a Do not lookup bufline if no scrollback
0a55d85 Tweaks to make bytes unsigned
e681d54 Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Nov 5, 2022, 9:14:48 AM11/5/22

to vim/vim, Subscribed

Rebased on latest master due to some changes in libvterm − all works fine, still ready for review.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Simon Sobisch

unread,

Nov 6, 2022, 5:02:51 AM11/6/22

to vim/vim, Subscribed

@brammool Looks good to me (but I'm only a mere user); is there any reason that this may not be merged?

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Nov 12, 2022, 6:56:11 PM11/12/22

to vim/vim, Subscribed

I see lots of details that need to be improved (code style, missing comments, etc.), but let's first look at the high level.

Have you discussed the changes in the libvterm directory with @leonerd ? Especially the change in sb_pushline, which is not backwards compatible. For a change local to Vim that's OK, but if the original libvterm would take over this change it probably has to be done differently.

I kind of lost track of what this PR is changing. The test should illustrate it, but it only seems to check that some command is wrapped at the right place. Why is there no check for what displays after restoring the width?

BTW: setting 'columns' in a test like this might cause trouble, better use a vertical split to reduce the width of the window. Then the terminal doesn't get resized.

exe "set columns=30"

There is no need for "exe" here.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Nov 13, 2022, 7:31:28 AM11/13/22

to vim/vim, Subscribed

At a high level, this PR takes terminal lines that are scrolled out of view and joins them together into a single buffer line when appropriate.

This is because terminal lines are often wrapped artificially due to their length, rather than to a line break. With these changes, resizing the vim buffer will properly display the wrapped lines, wapped words will no longer contain line breaks, etc. These are common occurrences as reported in #2865, i.e. if your normal buffer has line numbering then switching to normal view already causes resizes, paths can often be long and broken by artificial line breaks which mean they can’t be copied and pasted anymore, etc.

The test is checking that a command which is wrapped in the terminal (non-empty term_getline(…, 2)) is not wrapped in the buffer. Without the PR let lines = line('$') would return 2, not 1.

This PR is disconnecting the terminal lines (stored in tl_scrollback) from lines in the buffer, and adds mechanisms to get the corresponding buffer line from a scrollback line and conversely, typically to get terminal colour info in the normal view of a terminal buffer.

I have tried to get an opinion of @leonerd on these changes both here, on launchpad, and by email, but have never gotten a concrete answer. In essence all these changes are doing in the libvterm directory are adding a needed value to a callback, and correctly reordering the scrollrect and setlineinfo callbacks at the end of scroll() so the callbacks contain the values for the right lines.

I’ve really tried to keep the changes in libvterm to a minimum, and I’ve rebased them onto the libvterm codebase and offered them as a patch there. I’m not sure what else can be done.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Nov 13, 2022, 7:46:00 AM11/13/22

to vim/vim, Subscribed

Hi. I'm generally around and stuff. I really don't recall having seen much discussion on this before.

I'm often in two minds about libvterm from vim in particular. I recall youfolks took a copy of it then did lots of changes like making it C89 compatible by shuffling all the variable declarations and comments. That makes merging back upstream somewhat more difficult for me.

I'm happy to discuss on general concepts and my future plans going forward, especially in the scrollback management area. It's just a little more tricky to directly see how the exact changes here might apply.

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Simon Sobisch

unread,

Nov 13, 2022, 8:24:21 AM11/13/22

to vim/vim, Subscribed

I'm happy to discuss on general concepts and my future plans going forward, especially in the scrollback management area. It's just a little more tricky to directly see how the exact changes here might apply.

@leonerd what about https://bugs.launchpad.net/libvterm/+bug/1952530 and especially https://bugs.launchpad.net/libvterm/+bug/1952530/+attachment/5546517/+files/smaller-4-commits.patch ?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Nov 13, 2022, 8:40:01 AM11/13/22

to vim/vim, Subscribed

Yeah they all look good. Maybe can merge some of them together, there seems little point in keeping them all individually.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Nov 13, 2022, 8:43:43 AM11/13/22

to vim/vim, Subscribed

Turns out though that causes some test failures: https://bugs.launchpad.net/libvterm/+bug/1952530/comments/9

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Nov 13, 2022, 8:58:53 AM11/13/22

to vim/vim, Subscribed

But also I'll add a general note (I'm sure I've said this before): The best way to get features added or bugs fixed is just to explain it to me. Tell me in English what wants adding or what the problem is and I will go fix it. Starting off by sending me code-first is not the best way.

Perhaps that's unusual for opensource projects but I much prefer the discussion and interaction to happen first in English on general concepts and then for me to write the code afterwards. That always works a lot better for me than trying to understand what is being added or fixed, when the first interaction comes in the form of a code patch.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Simon Sobisch

unread,

Nov 13, 2022, 9:06:15 AM11/13/22

to vim/vim, Subscribed

Just to recheck @leonerd: the note at #8365 (comment) and at launchpad are clear, aren't they?

Tell me in English what wants adding or what the problem is and I will go fix it

So that means you're fixing the bug / implement that feature in libvterm now and we then can take that changes into vim?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Nov 13, 2022, 9:12:18 AM11/13/22

to vim/vim, Subscribed

Would an accurate description be: "sb_pushline needs to be told the lineinfo so it can know if the line has the continuation mark?"

If so, that's all the description I need, and I can go and implement that.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Nov 13, 2022, 9:34:19 AM11/13/22

to vim/vim, Subscribed

Paul Evans wrote:

> Hi. I'm generally around and stuff. I really don't recall having seen
> much discussion on this before.
>
> I'm often in two minds about libvterm from vim in particular. I recall
> youfolks took a copy of it then did lots of changes like making it C89
> compatible by shuffling all the variable declarations and comments.
> That makes merging back upstream somewhat more difficult for me.

Part of that has recently been reversed, now that we dropped support for
older MS-Windows compilers. Still, we can't use all of C99,
unfortunately, there are still some compilers missing parts of the
implementation. The struct initializations appears to be the biggest
one right now.

I have tried to sync back to a recent libvterm version, but it's very
tricky, since some things in the Vim fork of it are really bug fixes and
enhancements. And you have fixed bugs in a slightly different way. My
plan is to try to bring them back in sync a little piece at a time.
If you have suggestions of where it would help most, let me know.

> I'm happy to discuss on general concepts and my future plans going
> forward, especially in the scrollback management area. It's just a
> little more tricky to directly see how the exact changes here might
> apply.

I think the handling of terminal width changes is a basic libvterm
thing, which now pops up in Vim. That's why I thought you might have a
broader look at what solution works, especially when it involves
adjustments to the API.

From the Vim point of view, users may run some shell commands in the
terminal, which causes lines to wrap. When then making the terminal
window wider users will want to see those wraps adjust to the new width.
Thus there needs to be a difference between "soft wraps" (running into
the right border) and "hard wraps" (outputting a line break).
Would be nice to have a generic solution for that. But I know it might
not be so simple.

--
hundred-and-one symptoms of being an internet addict:
55. You ask your doctor to implant a gig in your brain.

/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Nov 13, 2022, 12:02:06 PM11/13/22

to vim/vim, Subscribed

@bram

Part of that has recently been reversed, now that we dropped support for older MS-Windows compilers. Still, we can't use all of C99, unfortunately, there are still some compilers missing parts of the implementation. The struct initializations appears to be the biggest one right now. I have tried to sync back to a recent libvterm version, but it's very tricky, since some things in the Vim fork of it are really bug fixes and enhancements. And you have fixed bugs in a slightly different way. My plan is to try to bring them back in sync a little piece at a time. If you have suggestions of where it would help most, let me know.

Ah - with my other hat on as a Perl core maintainer, I wonder if we could swap notes here. We recently decided to adopt C99, as basically enough of it is supported everywhere we care about. So now we get mixed declarations and code, bool, struct initializers, etc.. Those seem to work just fine for us - where are your tricky platforms for that?

I'm happy to discuss on general concepts and my future plans going forward, especially in the scrollback management area. It's just a little more tricky to directly see how the exact changes here might apply.

My overall plan is to migrate the "scrollback manager" parts out of pangoterm into their own optional side-layer in libvterm, as it seems everyone else who uses it basically just copypastes that part anyway. I might as well provide one standard one.

I think the handling of terminal width changes is a basic libvterm thing, which now pops up in Vim. That's why I thought you might have a broader look at what solution works, especially when it involves adjustments to the API. From the Vim point of view, users may run some shell commands in the terminal, which causes lines to wrap. When then making the terminal window wider users will want to see those wraps adjust to the new width. Thus there needs to be a difference between "soft wraps" (running into the right border) and "hard wraps" (outputting a line break). Would be nice to have a generic solution for that. But I know it might not be so simple.

libvterm 0.3 now has reflow on resize, but only within the "active" buffer, not the scrollback buffer. That's part of an ongoing design rethink to make that possible. It needs extensions and overlaps with the way the scrollback API works, so I thought I'd tackle it after I've moved it into libvterm itself.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Nov 13, 2022, 1:44:16 PM11/13/22

to vim/vim, Subscribed

Would an accurate description be: "sb_pushline needs to be told the lineinfo so it can know if the line has the continuation mark?"

If so, that's all the description I need, and I can go and implement that.

Yes that’s accurate. If you want to make a fix yourself I can certainly use that and rebase the changes on it.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 12, 2023, 10:07:55 AM7/12/23

to vim/vim, Push

@Cimbali pushed 11 commits.

8282f5f Add a test for terminal unwrapping
815ae56 Add VTermLineInfo* to libvterm sb_pushline callback
1afca75 Unwrap terminal lines in buffer
43e4aca Only un-scroll from buffer the lines that were added for snapshot
58587c1 Update limit_scrollback to take line continuations into account
8b2ac31 Parallel tracking of buffer lines and scrollback lines
863b124 Call scrollbackline_pos_in_buf() with absolute row number
e54bde7 Ensure the first line is not continuation on clear cmd
ab5a1f4 Do not lookup bufline if no scrollback
ef506d8 Tweaks to make bytes unsigned
bdea21d Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 12, 2023, 10:34:50 AM7/12/23

to vim/vim, Push

@Cimbali pushed 8 commits.

f4b1b54 Only un-scroll from buffer the lines that were added for snapshot
d290ba2 Update limit_scrollback to take line continuations into account
d189a50 Parallel tracking of buffer lines and scrollback lines
66c9021 Call scrollbackline_pos_in_buf() with absolute row number
16e7d92 Ensure the first line is not continuation on clear cmd
5456d76 Do not lookup bufline if no scrollback
8d497c3 Tweaks to make bytes unsigned
6d74d41 Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 12, 2023, 10:46:14 AM7/12/23

to vim/vim, Push

@Cimbali pushed 6 commits.

8cd51da Parallel tracking of buffer lines and scrollback lines
7551a06 Call scrollbackline_pos_in_buf() with absolute row number
3d70e2f Ensure the first line is not continuation on clear cmd
9c75750 Do not lookup bufline if no scrollback
38418ec Tweaks to make bytes unsigned
14f0f53 Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 12, 2023, 11:04:55 AM7/12/23

to vim/vim, Push

@Cimbali pushed 2 commits.

84457c0 Try to make test less flaky for CIs
9ab542b Ensure state_scrollrect_success is initialized

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 12, 2023, 11:05:32 AM7/12/23

to vim/vim, Subscribed

Mostly a straightforward rebase on current master, to handle conflicts created by changed indents in 032713f. Also fixed the curly bracket positions the CI complained about.

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Sergey Vlasov

unread,

Aug 5, 2023, 7:53:28 PM8/5/23

to vim/vim, Subscribed

@Cimbali, I tried the current PR. I'm not sure if this is intended but it seems switching between modes while resizing the window breaks rewrapping for old lines.

Steps:

vim -g --clean +'set lines=20 columns=22' +'term bash --noprofile --norc' +'only'
seq 100 | paste -sd+
Go to NORMAL
Change window width
Go to INSERT
Go to step 2

At the 2nd window resize, the old lines are no longer rewrapped.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Aug 7, 2023, 12:04:21 PM8/7/23

to vim/vim, Subscribed

Interactions with resize don’t work really well indeed, except for lines that are already off screen. That’s because these lines are still handled by libvterm, which doesn’t support reflowing. This PR adds support for reflowing in vim of terminal lines, so switching normal/insert without resizing works, and regardless of resizing for off-screen lines (which, granted, you can only see in NORMAL mode).

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 25, 2023, 8:19:47 AM9/25/23

to vim/vim, Push

@Cimbali pushed 12 commits.

bb54a21 Add a test for terminal unwrapping
739c2a7 Add VTermLineInfo* to libvterm sb_pushline callback
a83c2ba Unwrap terminal lines in buffer
ffa2a0b Only un-scroll from buffer the lines that were added for snapshot
8e47fb0 Update limit_scrollback to take line continuations into account
b521d33 Parallel tracking of buffer lines and scrollback lines
58a302c Call scrollbackline_pos_in_buf() with absolute row number
011ec4f Ensure the first line is not continuation on clear cmd
7e0de04 Do not lookup bufline if no scrollback
65edbf6 Tweaks to make bytes unsigned
48427e5 Ensure state_scrollrect_success is initialized
8d64945 Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 25, 2023, 8:35:24 AM9/25/23

to vim/vim, Push

@Cimbali pushed 1 commit.

390491b Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 25, 2023, 9:01:07 AM9/25/23

to vim/vim, Push

@Cimbali pushed 1 commit.

e45f8dc Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 25, 2023, 4:29:46 PM9/25/23

to vim/vim, Push

@Cimbali pushed 12 commits.

8f3ad84 Add a test for terminal unwrapping
b135d63 Add VTermLineInfo* to libvterm sb_pushline callback
a475f96 Unwrap terminal lines in buffer
ba61d70 Only un-scroll from buffer the lines that were added for snapshot
62083d4 Update limit_scrollback to take line continuations into account
9a1b3d9 Parallel tracking of buffer lines and scrollback lines
7c52165 Call scrollbackline_pos_in_buf() with absolute row number
4d23f5f Ensure the first line is not continuation on clear cmd
572f928 Do not lookup bufline if no scrollback
759f58a Tweaks to make bytes unsigned
78d947b Ensure state_scrollrect_success is initialized
ef4172b Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Simon Leischnig

unread,

Jul 15, 2024, 3:37:55 PM7/15/24

to vim/vim, Subscribed

Is there any perspective to this moving forward?
Could maybe a feature change/add and a structured description of intended behavior help? @Cimbali

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Jul 15, 2024, 4:04:37 PM7/15/24

to vim/vim, Subscribed

libvterm has optional reflow now, since some time ago.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 16, 2024, 6:50:21 AM7/16/24

to vim/vim, Subscribed

This PR has never been about reflow in an active terminal session, but about reflowing terminal contents once passed on to vim.

We’re still stuck at the same point @simlei:

either libvterm adds an info to its pushline callback (whether line breaks are “soft”, i.e. line wraps, or “hard”, i.e. line ends).
or vim’s copy of libvterm adds it without it existing upstream

There’s just no way to reflow without knowing which line breaks can be moved and which can’t.

So far (1) seemed the preferred option, with @leonerd stating in this thread he preferred implementing his own version of this requirement over the patches I sent, to which I replied I’ll happily rebase on that work when made available. That was 20 months ago.

I think the correct way forward is to set a deadline by which we fall back to option (2) if this current reminder (and any potential future ones) keep remaining unaddressed.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Jul 16, 2024, 7:14:38 AM7/16/24

to vim/vim, Subscribed

I'm sure I could look into adding something in libvterm for that then, yes.

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Jul 16, 2024, 8:16:22 AM7/16/24

to vim/vim, Subscribed

Right - thinking out loud this is slightly nontrivial. If there were no API compat guaratnees at all, then I could simply add a VTermLineInfo * parameter to sb_pushline and all would be well. But I can't because that would break older code.

I'm in a dilemma of

Major version bump, up to v0.4, just for this one one change. If I'm going to bump that number I feel I should take the opportunity to change a lot more stuff while I'm there and that makes it take a lot longer.
Temporarily add an API to set a different callback that takes that extra argument, as a workaround for not changing it currently. You'll have to call a different function to set the pushline callback with a different signature.

It leads to a messier API but it's only temporary, so I'm going to go for option 2.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Simon Leischnig

unread,

Jul 16, 2024, 8:49:11 AM7/16/24

to vim/vim, Subscribed

Let me know if I can help, however I have very limited knowledge of the vim
codebase.

On Tue, 16 Jul 2024, 14:14 Paul Evans ***@***.***> wrote:

> Right - thinking out loud this is slightly nontrivial. If there were no
> API compat guaratnees at all, then I could simply add a VTermLineInfo *
> parameter to sb_pushline and all would be well. But I can't because that
> would break older code.
>
> I'm in a dilemma of
>

> 1. Major version bump, up to v0.4, just for this one one change. If

> I'm going to bump that number I feel I should take the opportunity to
> change a lot more stuff while I'm there and that makes it take a lot longer.

> 2. Temporarily add an API to set a different callback that takes that

> extra argument, as a workaround for not changing it currently. You'll have
> to call a different function to set the pushline callback with a different
> signature.
>
> It leads to a messier API but it's only temporary, so I'm going to go for
> option 2.
>
> —
> Reply to this email directly, view it on GitHub

> <https://github.com/vim/vim/pull/8365#issuecomment-2230739575>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAHP225FSSUTWI3PSE2ZTSTZMUFDBAVCNFSM46RA3CN2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMRTGA3TGOJVG42Q>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Jul 16, 2024, 9:05:00 AM7/16/24

to vim/vim, Subscribed

Let me know if I can help, however I have very limited knowledge of the vim codebase.

Ah this is all in libvterm, it's really not directly vim-related code at all.

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Paul Evans

unread,

Jul 16, 2024, 10:46:43 AM7/16/24

to vim/vim, Subscribed

either libvterm adds an info to its pushline callback (whether line breaks are “soft”, i.e. line wraps, or “hard”, i.e. line ends).

Right then. Took me a while to remember to getting around to do it, but this is now done in latest source on bzr. What would be a good way to deliver this for your use? I'd like you to try it out a bit first before I call it the official 0.3.4 version tarball, but I could pack you a pre-release version, or make a .diff out of the relevant commits... or something else?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 16, 2024, 4:26:56 PM7/16/24

to vim/vim, Subscribed

Thanks a lot @leonerd ! I’ve fetch the commits from bazaar and am playing around with it. It seems to work well as a drop-in replacement for what I had written, but I need to do some further testing.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 17, 2024, 3:54:45 PM7/17/24

to vim/vim, Push

@Cimbali pushed 14 commits.

6be93f3 Add a test for terminal unwrapping
fb69702 Add a 'premove' state-layer callback to inform the higher layer before scroll-based moves happen
8db87a1 Use premove rather than start of moverect to implement scrollback push in screen layer
3cb06dd Added sb_pushline4 screen callback, which adds an extra 'continuation' argument
91483a2 Capture t/69screen_pushline.test
1cd2d5c Hook into continuation-aware pushline callback
4dc2c2c Unwrap terminal lines in buffer
8d62500 Only un-scroll from buffer the lines that were added for snapshot
75e23ce Update limit_scrollback to take line continuations into account
53cbc1e Parallel tracking of buffer lines and scrollback lines
c430180 Call scrollbackline_pos_in_buf() with absolute row number
feb5eff Do not lookup bufline if no scrollback
fff1e9a Tweaks to make bytes unsigned
ee0a955 Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 18, 2024, 3:00:51 AM7/18/24

to vim/vim, Push

@Cimbali pushed 1 commit.

a85ba90 Add new test file to Filelist

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 18, 2024, 3:06:37 AM7/18/24

to vim/vim, Push

@Cimbali pushed 1 commit.

d383963 Mark local functions static

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 18, 2024, 5:17:56 AM7/18/24

to vim/vim, Push

@Cimbali pushed 1 commit.

677b216 Nit mark void *user as unused

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jul 25, 2024, 11:11:37 AM7/25/24

to vim/vim, Subscribed

I’ve confirmed this doesn’t work on Windows for some reason. Will have to get my hands on an appropriate setup to build where I can investigate properly why that is (appveyor rdp is a fallback but a bit too limited at this stage).

I can’t reproduce the linux uchar test issue/flakiness however.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Simon Leischnig

unread,

Jul 25, 2024, 6:24:36 PM7/25/24

to vim/vim, Subscribed

I have a Windows PC and can help with testing. Can get any build to work in
wsl

On Thu, 25 Jul 2024, 17:10 Cimbali ***@***.***> wrote:

> I’ve confirmed this doesn’t work on Windows for some reason. Will have to
> get my hands on an appropriate setup to build where I can investigate
> properly why that is (appveyor rdp is a fallback but a bit too limited at
> this stage).
>
> I can’t reproduce the linux uchar test issue/flakiness however.
>
> —
> Reply to this email directly, view it on GitHub

> <https://github.com/vim/vim/pull/8365#issuecomment-2250630072>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAHP222XHT3Y7DZGRUC4G4DZOEIP7AVCNFSM46RA3CN2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMRVGA3DGMBQG4ZA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Simon Leischnig

unread,

Oct 28, 2024, 4:09:17 PM10/28/24

to vim/vim, Subscribed

I'd like to again offer my support in debugging this issue. Have a windows PC. gdb noob though; instructions required.

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Oct 29, 2024, 6:35:22 AM10/29/24

to vim/vim, Subscribed

Thanks @simlei. I think good first steps would be compiling and running the unit tests, to reproduce. Then figuring out why the failing unit test fails. Probably adding stderr printfs in handle_pushline and add_scrollback_line_to_buffer and redirecting that to a log with vim 2> log to understand what’s going on, whether continuation flags are correctly received, if so why pasting lines together doesn’t work, and so on.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Simon Leischnig

unread,

Oct 29, 2024, 12:11:52 PM10/29/24

to vim/vim, Subscribed

Ok, will get a setup running on the weekend and ping you back when I get
stumped ok?

On Tue, Oct 29, 2024 at 11:34 AM Cimbali ***@***.***> wrote:

> Thanks @simlei <https://github.com/simlei>. I think good first steps

> would be compiling and running the unit tests, to reproduce. Then figuring
> out why the failing unit test fails. Probably adding stderr printfs in
> handle_pushline and add_scrollback_line_to_buffer and redirecting that to
> a log with vim 2> log to understand what’s going on, whether continuation
> flags are correctly received, if so why pasting lines together doesn’t
> work, and so on.
>
> —
> Reply to this email directly, view it on GitHub

> <https://github.com/vim/vim/pull/8365#issuecomment-2443842521>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAHP223VGNSJZGFEGC6OUZDZ55QEHAVCNFSM46RA3CN2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TENBUGM4DIMRVGIYQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Lucas Pütz

unread,

Feb 11, 2025, 8:41:47 AMFeb 11

to vim/vim, Subscribed

@Cimbali Can you give an update on the current state of this PR?

Also, is there any way to assist you? I would be happy to help :)

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Simon Leischnig

unread,

Feb 11, 2025, 9:07:47 AMFeb 11

to vim/vim, Subscribed

I have previously offered to have a look too but ultimately have not found
the time yet. However during 10-15 March this year might be able to

On Tue, 11 Feb 2025, 14:41 Lucas Pütz ***@***.***> wrote:

> @Cimbali <https://github.com/Cimbali> Can you give an update on the

> current state of this PR?
>
> Also, is there any way to assist you? I would be happy to help :)
>
> —
> Reply to this email directly, view it on GitHub

> <https://github.com/vim/vim/pull/8365#issuecomment-2650861454>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAHP225SQDWBZWA62KNIIXL2PH4XJAVCNFSM46RA3CN2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TENRVGA4DMMJUGU2A>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Apr 15, 2025, 8:14:23 AMApr 15

to vim/vim, Push

@Cimbali pushed 17 commits.

d2dbcf9 Add a test for terminal unwrapping
feb83a5 Add a 'premove' state-layer callback to inform the higher layer before scroll-based moves happen
7447203 Use premove rather than start of moverect to implement scrollback push in screen layer
2c43870 Added sb_pushline4 screen callback, which adds an extra 'continuation' argument
8bfd88a Capture t/69screen_pushline.test
2b3702f Hook into continuation-aware pushline callback
f25d455 Unwrap terminal lines in buffer
3c18c3c Only un-scroll from buffer the lines that were added for snapshot
c09696c Update limit_scrollback to take line continuations into account
34d4885 Parallel tracking of buffer lines and scrollback lines
728f128 Call scrollbackline_pos_in_buf() with absolute row number
9f3fa3f Do not lookup bufline if no scrollback
dc34d8e Tweaks to make bytes unsigned
b0f828c Try to make test less flaky for CIs
1b543c5 Add new test file to Filelist
feaf930 Mark local functions static
8e6c19f Nit mark void *user as unused

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Jun 23, 2025, 6:19:27 PMJun 23

to vim/vim, Push

@Cimbali pushed 12 commits.

6be93f3 Add a test for terminal unwrapping
47a77da Add VTermLineInfo* to libvterm sb_pushline callback
7081768 Unwrap terminal lines in buffer
1c6cc35 Only un-scroll from buffer the lines that were added for snapshot
151ae49 Update limit_scrollback to take line continuations into account
8654ba0 Parallel tracking of buffer lines and scrollback lines
6d4c90f Call scrollbackline_pos_in_buf() with absolute row number
027c26c Ensure the first line is not continuation on clear cmd
9e33745 Do not lookup bufline if no scrollback
c634e58 Tweaks to make bytes unsigned
01b9919 Ensure state_scrollrect_success is initialized
6d96c4d Try to make test less flaky for CIs

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Simon Sobisch

unread,

Sep 15, 2025, 2:13:14 AMSep 15

to vim/vim, Subscribed

GitMensch left a comment (vim/vim#8365)

Just to check where the state is:

https://code.launchpad.net/~libvterm/libvterm/trunk contains necessary upstream changes which will go into a 0.3.4 release once testing is finished
this PR includes those changes here
this PR solves the "terminal buffer never unwraps" issue on GNU/Linux
while this PR passes existing tests on a local GNU/Linux machine, it doesn't anywhere else including the CI

Is that correct?
Also: Would there be the need to change Vim runtime files to make that usable (for example in TermDebug)?

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 15, 2025, 5:22:05 AMSep 15

to vim/vim, Subscribed

Cimbali left a comment (vim/vim#8365)

That is correct, but previously most CIs used to run - all except Windows. I’ve pushed rebases without checking CIs it’s probably not compliant with recent CI changes.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Cimbali

unread,

Sep 15, 2025, 5:44:41 AMSep 15

to vim/vim, Push

@Cimbali pushed 17 commits.

4c586c9 Add a test for terminal unwrapping
a73e3bc Add a 'premove' state-layer callback to inform the higher layer before scroll-based moves happen
faa9928 Use premove rather than start of moverect to implement scrollback push in screen layer
34b162b Added sb_pushline4 screen callback, which adds an extra 'continuation' argument
616e2fd Capture t/69screen_pushline.test
1f65e52 Hook into continuation-aware pushline callback
43cef49 Unwrap terminal lines in buffer
93f636e Only un-scroll from buffer the lines that were added for snapshot
dc7e8fa Update limit_scrollback to take line continuations into account
5d5e44d Parallel tracking of buffer lines and scrollback lines
274be10 Call scrollbackline_pos_in_buf() with absolute row number
51b1fd2 Do not lookup bufline if no scrollback
3c78935 Tweaks to make bytes unsigned
6279c9d Try to make test less flaky for CIs
a4251c4 Add new test file to Filelist
35cd41d Mark local functions static
67a9165 Nit mark void *user as unused

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward

[vim/vim] Unwrap terminal buffer (#8365)

Cimbali

You can view, comment on, or merge this pull request online at:

Commit Summary

File Changes

Patch Links:

Cimbali

James McCoy

Cimbali

Cimbali

K.Takata

Cimbali

codecov[bot]

Codecov Report

Cimbali

Cimbali

Paul Evans

Cimbali

Cimbali

Bram Moolenaar

Cimbali

Cimbali

Cimbali

Simon Sobisch

Cimbali

Cimbali

Cimbali

damnskippy

Cimbali

Paul Evans

Cimbali

Paul Evans

Cimbali

Sergey Vlasov

Cimbali

Sergey Vlasov

Simon Sobisch

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Simon Sobisch

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Simon Sobisch

Bram Moolenaar

Cimbali

Paul Evans

Simon Sobisch

Paul Evans

Paul Evans

Paul Evans

Simon Sobisch

Paul Evans

Bram Moolenaar

Paul Evans

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Sergey Vlasov

Cimbali

Cimbali

Cimbali

Cimbali

Cimbali

Simon Leischnig

Paul Evans