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

Rightmost column of text is invisible on moar #18115

Closed
lapo-luchini opened this issue Oct 27, 2024 · 19 comments
Closed

Rightmost column of text is invisible on moar #18115

lapo-luchini opened this issue Oct 27, 2024 · 19 comments
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons.

Comments

@lapo-luchini
Copy link

Windows Terminal version

1.21.2911.0

Windows build number

10.0.26120.0

Other Software

moar 1.27.2 installed via go install github.com/walles/moar@latest (issue 250 there)

Steps to reproduce

This happens both on WSL:

go install github.com/walles/moar@latest
seq -s '-' 10 99 | moar -wrap

and also on cmd:

go install github.com/walles/moar@latest
echo 10-11-12-13-14-15-16-17-18-19-20-21-22-23-24-25-26-27-28-29-30-31-32-33-34-35-36-37-38-39-40-41-42-43-44-45-46-47-48-49-50-51-52-53-54-55-56-57-58-59-60-61-62-63-64-65-66-67-68-69-70-71-72-73-74-75-76-77-78-79-80-81-82-83-84-85-86-87-88-89-90-91-92-93-94-95-96-97-98-99 | moar -wrap

Expected Behavior

Line breaking at the end of the terminal width, continuing on next line:

┌────────────────────────┐ screen width
10-11-12-13-14-15-16-17-18
-19-20-21-22-23-24-25-26-2
7-28-29-30…

Actual Behavior

Last column is blank and is not shown:
e.g.

┌────────────────────────┐ screen width
10-11-12-13-14-15-16-17-1
-19-20-21-22-23-24-25-26-
7-28-29-30…
@lapo-luchini lapo-luchini added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 27, 2024
@lapo-luchini lapo-luchini changed the title Rightmost column of terxt invisible on moar Rightmost column of text is invisible on moar Oct 27, 2024
@walles
Copy link

walles commented Oct 30, 2024

FWIW, moar shows the last column of text fine on all tested non-ConPTY terminals.

Test results for moar showing the last column

  • ❌ Windows Terminal (on Windows)
  • ❌ VSCode terminal on Windows. On macOS, the VSCode terminal works fine.
  • ✅ VSCode terminal on macOS. On Windows, the VSCode terminal does not show the last column.
  • ✅ iTerm2 on macOS
  • ✅ macOS Terminal
  • ✅ Hyper on macOS
  • ✅ KiTTY on macOS
  • ✅ WezTerm on macOS
  • ✅ MobaXTerm on Windows: Wrapping long words eats up one character walles/moar#250 (comment)

@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

This will be very helpful for a differential diagnosis:

Does moar work properly in WSL under Windows Terminal and VSCode?

If so: there's a chance that it has--or is using a library which has--Windows platform-specific code.

If it does, I suspect that it is--or that library is--using GetConsoleScreenBufferInfo(Ex) and treating srWindow as exclusive when it should be inclusive.

@zadjii-msft
Copy link
Member

Also,

  • Does it work in conhost.exe (not the Terminal)
  • Does it work on Terminal Preview (1.22+, where there was a bit of a rewite of ConPTY)

@walles
Copy link

walles commented Oct 30, 2024

If so: there's a chance that it has--or is using a library which has--Windows platform-specific code.

moar is using Go's term.GetSize() function, ending up in here: https://cs.opensource.google/go/x/term/+/refs/tags/v0.25.0:term_windows.go;l=48-54

func getSize(fd int) (width, height int, err error) {
	var info windows.ConsoleScreenBufferInfo
	if err := windows.GetConsoleScreenBufferInfo(windows.Handle(fd), &info); err != nil {
		return 0, 0, err
	}
	return int(info.Window.Right - info.Window.Left + 1), int(info.Window.Bottom - info.Window.Top + 1), nil
}

If it does, I suspect that it is--or that library is--using GetConsoleScreenBufferInfo(Ex) and treating srWindow as exclusive when it should be inclusive.

Let's say the window is 100 chars wide.

If moar thought the windows was 99 chars wide, moar would wrap too early.

But what's happening is that the last char on the line goes missing.

So I believe moar is getting the right information, it just fails to write chars to the last column (with ConPTY).

@DHowett ^

@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

Ugh, I missed your comment about WSL earlier. I'm sorry.

@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

@zadjii-msft It reproduces in preview and canary, as well as conhost.

Image

Image

In both cases I used a block selection to highlight the rightmost edge.

@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

Got it!

This is what moar is printing to the console:

␛[1;1H␛[m␛[38;2;104;104;104m  1 ␛[38;2;208;208;208m10-11-12-13-14-15-16-17-18-19-20-21-22-23-24-25-26-27-28-29-30-31-32-33-34-35-36-37-38-39-40-41-42-43-44-45-46-47-48␛[m␛[K  ␛[m    ␛[38;2;208;208;208m-49-50-51-52-53-54-55-56-57-58-59-60-61-62-63-64-65-66-67-68-69-70-71-72-73-74-75-76-77-78-79-80-81-82-83-84-85-86-8␛[m␛[K  ␛[m    ␛[38;2;208;208;208m7-88-89-90-91-92-93-94-95-96-97-98-99␛[m␛[K  ␛[m␛[7m---␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[K  ␛[m␛[7m1 line  100%  Press 'ESC' / 'q' to exit, '/' to search, '?' for help                                                    ␛[m␛[K

Zooming in on the end of the first line (which is 46-47-48 in my case), I see this:

46-47-48␛[m␛[K  ␛[m
       |   |   \- space
       |   \- clear to end of line
       \- fill the rightmost column

By design, emitting a \e[K (EL 0 Erase to Right) when the cursor is in the deferred last column position clears the last column. 🙂

Now to figure out why moar is emitting \e[K in the deferred wrap position!

EDIT to add NOTE: this does not require ConPTY, and does not appear to be a ConPTY issue. This appears to be an application issue (at the moment.)

@walles
Copy link

walles commented Oct 30, 2024

By design, emitting a \e[K (EL 0 Erase to Right) when the cursor is in the deferred last column position clears the last column. 🙂

I will change this behavior in moar.

Out of curiosity, why this design? When all other terminals seem to handle this differently?

@walles
Copy link

walles commented Oct 30, 2024

Now to figure out why moar is emitting \e[K in the deferred wrap position!

Instead of clearing the screen at the start of a redraw, moar clears to EOL after it's done with rendering all chars on that line.

The hope (not measured) is that this might avoid some flickering during redraws.

walles added a commit to walles/moar that referenced this issue Oct 30, 2024
Before this change, on Windows, the rightmost column of the screen would
be cut off and not visible.

With this change in place, that column should now be visible.

Fixes #250.

Ref:
microsoft/terminal#18115 (comment)
@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

Out of curiosity, why this design?

That's a good question, but I should note that xterm does the same thing:

Image

The former libvte (gnome-terminal et al) maintainer filed a bug on us requesting that we deviate from STD 070, and James--who I look to as the paragon of all terminal emulation knowledge--had this to say: #3177 (comment)

He followed up with #14936, which pertains only to the delayed wrap state and not the last column.

@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

TL;DR: this is supported by standard behavior in either direction.

@j4james likely knows much more than I do about it. 🙂

@j4james
Copy link
Collaborator

j4james commented Oct 30, 2024

We've tried to follow the spec as closely as possible (documented in STD 070, pages D-13 and D-14). I think the only intentional deviation is with the tab control, where the behavior was changed in later DEC terminals (in that case we're matching the most recent DEC implementation, rather than the older spec definition).

I believe this is exactly the same behavior as XTerm, and last I checked, MLTerm and the Linux console were also following the spec quite closely. We're definitely not the only terminal that handles EL this way. So if you're seeing something different in XTerm, it's probably worth investigating whether there is some other factor involved.

@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

Thanks!

And with that, I'll close this one as By Design.

@DHowett DHowett closed this as not planned Won't fix, can't repro, duplicate, stale Oct 30, 2024
@DHowett DHowett added the Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons. label Oct 30, 2024
@DHowett
Copy link
Member

DHowett commented Oct 30, 2024

(Also, I think I need to install moar on all of my machines so thank you for that)

@o-sdn-o
Copy link

o-sdn-o commented Oct 30, 2024

It seems to me that with such a design, a console application that outputs via a serial port and has no way to track the size of the terminal will inevitably periodically lose the last character. Moreover, with a terminal size of 1xN, the last character will always be lost.

@DHowett
Copy link
Member

DHowett commented Oct 31, 2024

@o-sdn-o Literal serial terminals act like this, so.

@walles
Copy link

walles commented Oct 31, 2024

if you're seeing something different in XTerm

I haven't tried XTerm, but I did produce this list (before I just changed moar's behavior):

Test results for moar showing the last column

  • ❌ Windows Terminal (on Windows)
  • ❌ VSCode terminal on Windows. On macOS, the VSCode terminal works fine.
  • ✅ VSCode terminal on macOS. On Windows, the VSCode terminal does not show the last column.
  • ✅ iTerm2 on macOS
  • ✅ macOS Terminal
  • ✅ Hyper on macOS
  • ✅ KiTTY on macOS
  • ✅ WezTerm on macOS
  • ✅ MobaXTerm on Windows: Wrapping long words eats up one character walles/moar#250 (comment)

I do appreciate the thorough response!

@o-sdn-o
Copy link

o-sdn-o commented Oct 31, 2024

With this design, supporting the \e[K command is completely pointless. Applications that can track the terminal size can easily erase the rest of the line using spaces, and this will be even more efficient. Other applications cannot rely on this vt-command at all because of the risk of losing the last character.

@j4james
Copy link
Collaborator

j4james commented Oct 31, 2024

@o-sdn-o It's worked this way for more than 40 years - I think it's a little late to decide you don't like the design. But if you want to invent a new protocol, maybe propose something on terminal-wg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons.
Projects
None yet
Development

No branches or pull requests

6 participants