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

x/crypto/ssh/terminal: GetSize returns incorrect results #27743

Closed
egazz opened this issue Sep 18, 2018 · 6 comments
Closed

x/crypto/ssh/terminal: GetSize returns incorrect results #27743

egazz opened this issue Sep 18, 2018 · 6 comments

Comments

@egazz
Copy link

@egazz egazz commented Sep 18, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.11 windows/amd64

Does this issue reproduce with the latest release?

Yes (since as far as I know this is the latest release).

What operating system and processor architecture are you using (go env)?

Windows 10, amd64.

What did you do?

package main

import (
	"fmt"
	"os"
	"golang.org/x/crypto/ssh/terminal"
)

func main() {

	termID := int(os.Stdout.Fd())
	termWidth, termHeight, termSizeErr := terminal.GetSize(termID)
	
	if termSizeErr != nil {
	
		panic(termSizeErr)
		
	}
	
	fmt.Printf("Width: %v, height: %v", termWidth, termHeight)

}

What did you expect to see?

Accurate height and width of the terminal.

What did you see instead?

Height is way off. My terminal window is 30 lines tall, but this returns a height of 9001. Width was correct, however.

In addition, I noticed that this issue remains. On Windows, getting the terminal handle via os.Stdin.Fd() results in an error, while os.Stdout.Fd() results in the above. However, on Linux, using Stdin works properly.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Sep 22, 2018

I think, it kind of works

image

There are many definitions of terminal height and width.

I looked at the code, and GetSize uses GetConsoleScreenBufferInfo Windows API. What should this package use instead? Why?

Thank you.

Alex

@egazz
Copy link
Author

@egazz egazz commented Sep 24, 2018

Hi, thanks for the reply.

I will say what I expected was the window size. I can imagine scenarios where one may need both, but for a basic console app, I think size of the window, i.e. the viewable area, is more important.

For example, if I wanted to know how much space I have for writing text, I'd need to know the dimensions (same thing for clearing the screen, unless there's a method that I haven't found yet....) The same is true for determining whether things need to be redrawn if the terminal window is resized.

I noticed this library uses syscall to reference TIOCGWINSZ, which according to this is part of the Linux kernel at least. I don't know if it'd work on Windows, though.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Sep 24, 2018

I don't know if it'd work on Windows, though.

That code does not run on Windows - note how first line of that source file says

// +build !windows

Alex

@egazz
Copy link
Author

@egazz egazz commented Sep 26, 2018

That was my concern. It looks, then, like the only way to get this information is sending a command such as stty via os.Stdin?

But back to my original issue, at the least it seems like the documentation should make it clearer that GetSize in fact gets the buffer size, not the window size. It's also worth noting that this behavior is different on Linux, where on at least some terminal emulators (such as urxvt) it returns the window size.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Sep 30, 2018

It looks, then, like the only way to get this information is sending a command such as stty via os.Stdin?

There is no stty command on Windows, if that is what you are suggesting.

Alex

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 23, 2019

Change https://golang.org/cl/163538 mentions this issue: ssh/terminal: fix GetSize on Windows

bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
bored-engineer pushed a commit to bored-engineer/ssh that referenced this issue Oct 13, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
umk pushed a commit to umk/go-terminal that referenced this issue Nov 10, 2019
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Feb 28, 2020
gopherbot pushed a commit to golang/term that referenced this issue Oct 16, 2020
Return window size instead of buffer size.

Fixes golang/go#27743

Change-Id: Ib1cd249f5680d86d505032e51d9102c2718ddf6f
Reviewed-on: https://go-review.googlesource.com/c/163538
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.