Extra line at the bottom of tmux

[ferdinandyb]

Contour Terminal version

latest master

Installer source

Github: source code cloned

Operating System

Ubuntu 22.04

Architecture

x86-64

Other Software

latest master tmux

Steps to reproduce

open tmux and observe the extra line at the bottom:

Expected Behavior

No response

Actual Behavior

There's an extra line which shouldn't be.

Additional notes

No response

[Yaraslaut]

Might be connected to #967

[christianparpart]

I tried looking at this days ago, because it happens to me as well (I tried master. branch, but the problem should be there on older tmux versions as well).

The reason for that is the use of margin VT sequences inside of contour.

A quick workaround for those users affected by this is to disable margins (either explicitly via setting, or by downgrading VT to e.g. VT340, IIRC)

What is happening here is, that when the status line at the bottom is to be written and the last character on the bottom most right column is being written, a line wrap occurs before writing that character, which makes it look like the status line takes two lines and the second line contains only the last character on the very left of that line.

It is very long time ago when we touched this logic, so I wonder why we didn't notice this earlier. We have test cases for auto-wrap as well as for margins. There might be a bug, though, I doubt it. Maybe we're missing a special test case (which should be added, once we found the root cause here) :)

EDIT: we should apply the fix on master branch as well as the 0.3.x branch :)

[j4james]

I'm not sure what the specific issue is with tmux, but I'm aware of a couple of bugs in Contour's margin implementation which may be relevant.

1. If you've set a top margin, CUU should not be able to cross that margin, unless the cursor is already above the top.
2. If you've set a left margin, CUB should not be able to cross that margin, unless the cursor is already to the left of it.
3. Applying DECSTBM with no parameters should set the vertical margins to the full height of the display. More specifically, the default for the first parameter should be 1, and the default for the second parameter should be the last line of the display.
4. Applying DECSLRM with no parameters should set the horizontal margins to the full width of the display. Again, the first parameter should default to 1, and the second should default to the last column of the display.

I haven't tested on master though - this was seen in version 0.3.12.262 - so it's possible these issues have already been fixed.

[christianparpart]

Oh, many thanks, @j4james, for your valuable feedback. I'll look into each individually ASAP. I was late in responding because I just came back from a tiny family trip. Thanks. :)

this was seen in version 0.3.12.262

We didn't change semantics or add crazy stuff post 0.3.12 on the backend side just yet, so it's likely some of these still apply to master as well.

However, what I noticed, while trying to trace it down, is, that the line problem seems to be because of the very last character to be written on the status line, is actually wrapped around to the next line, making the tmux statusline look like 2.

[christianparpart]

If you've set a top margin, CUU should not be able to cross that margin, unless the cursor is already above the top.

Sorry, p.s. here... :)

I just checked the spec on at least a bit of what you said:

If you've set a top margin, CUU should not be able to cross that margin, unless the cursor is already above the top.

As per https://vt100.net/docs/vt510-rm/CUU.html - that seems to be how it shall be implemented. Or did you just wrote how it should be and we're not adhering to this :)

If you've set a left margin, CUB should not be able to cross that margin, unless the cursor is already to the left of it.

https://vt100.net/docs/vt510-rm/CUB.html

Here, the spec actually quotes: The cursor stops at the left border of the page, so I might very well not implement it correctly, because I might have thought about "margin" here rather than "border".

DECSTBM / DECSLRM defaults

Have been both fixed by @Utkarsh-khambra about 3 weeks ago. #1165 :-)

[j4james]

Or did you just wrote how it should be and we're not adhering to this :)

Sorry, I wasn't very clear. I was trying to say that you're not adhering to that requirement. I've also realised now that the bug is more subtle than I first thought for the CUU case. It only fails when the cursor position is exactly on the boundary. In the test case below, the cursor should be clamped at row 10, but it moves all the way up to the top row.

printf "\e[10;20r"     # Set margins to 10..20
printf "\e[10H\e[10A"  # Place cursor on row 10 and try to move up 10 rows

Here, the spec actually quotes: The cursor stops at the left border of the page, so I might very well not implement it correctly, because I might have thought about "margin" here rather than "border".

That VT510 manual isn't great - the DEC STD-070 reference gives much more detail - but your understanding was correct. It's meant to stop at the left margin if there is one, but Contour isn't doing that. In the test case below, the cursor should clamp at column 10, but it moves to the first column.

printf "\e[?69h"          # Enable left right margin mode
printf "\e[10;60s"        # Set margins to 10..60
printf "\e[12;40H\e[40D"  # Place cursor in column 40 and try to move left 40 columns

DECSTBM / DECSLRM defaults

Have been both fixed by @Utkarsh-khambra about 3 weeks ago. #1165 :-)

Excellent! I should really be testing the master branch.

[christianparpart]

Wow. Many thanks, @j4james !

[christianparpart]

@ferdinandyb if you could still reproduce your problem from the top post, I might have just fixed it with the last PR merge linked above. At least I cannot repro it anymore. Can you confirm?

@j4james I'm not pretending to be 100% compatible wrt. margins just yet, I need some more time to go through more VT sequences that change cursor positioning and should properly handle margins. I took care of it, in fact, in the past, but apparently not the way it should have been. I'm adapting the test cases accordingly in future commits then. Thanks again, for your testings and patience. :)

[ferdinandyb]

On Thu Sep 21, 2023 at 09:23, Christian Parpart wrote: @ferdinandyb if you could still reproduce your problem from the top post, I might have just fixed it with the last PR merge linked above. At least I cannot repro it anymore. Can you confirm?

No, I'm not seeing this anymore, although I tried to repro and couldn't so ... No idea how, but it is solved :) Thanks!

[christianparpart]

@ferdinandyb well. I fixed the reported issues from @j4james. So looks like it's save to close this ticket then. Many thanks for testing (and your patience) :)