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

Unix Console SetWindowSize implementation #96208

Closed
tmds opened this issue Dec 20, 2023 · 3 comments · Fixed by #100272
Closed

Unix Console SetWindowSize implementation #96208

tmds opened this issue Dec 20, 2023 · 3 comments · Fixed by #100272
Assignees
Labels
area-System.Console blocking-release help wanted [up-for-grabs] Good issue for external contributors in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@tmds
Copy link
Member

tmds commented Dec 20, 2023

In #75824 support was added for Console.SetWindowSize on Unix by using ioctl TIOCSWINSZ.

As I mentioned in #95654 (comment), I believe this is for informing the kernel about the actual size, and not for requesting a size.

This matches the behavior mentioned by @adamsitnik in this comment and the referenced stack overflow answer.

#75824 (comment) seems to sort of work because the kernel is told the terminal has a size of (80, 30). nvim gets that size when it asks the terminal size, and it then renders in that part of the terminal window only.

However, the window that shows the terminal doesn't show the terminal size changed.

I don't think this matches the Windows semantics, where (I assume) the window would resize or show scrollbars.

Based on this, I think we should not use TIOCSWINSZ to implement Console.SetWindowSize on Unix.

cc @adamsitnik @danmoseley @stephentoub @dotnet/area-system-console

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 20, 2023
@ghost
Copy link

ghost commented Dec 20, 2023

Tagging subscribers to this area: @dotnet/area-system-console
See info in area-owners.md if you want to be subscribed.

Issue Details

In #75824 support was added for Console.SetWindowSize on Unix by using ioctl TIOCSWINSZ.

As I mentioned in #95654 (comment), I believe this is for informing the kernel about the actual size, and not for requesting a size.

This matches the behavior mentioned by @adamsitnik in this comment and the referenced stack overflow answer.

#75824 (comment) seems to sort of work because the kernel is told the terminal has a size of (80, 30). nvim gets that size when it asks the terminal size, and it then renders in that part of the terminal window only.

However, the window that shows the terminal doesn't show the terminal size changed.

I don't think this matches the Windows semantics, where (I assume) the window would resize or show scrollbars.

Based on this, I think we should not use TIOCSWINSZ to implement Console.SetWindowSize on Unix.

cc @adamsitnik @danmoseley @stephentoub @dotnet/area-system-console

Author: tmds
Assignees: -
Labels:

area-System.Console, untriaged

Milestone: -

@tmds
Copy link
Member Author

tmds commented Feb 8, 2024

Based on the previous comment, I think we should revert #75824.
This means that the APIs will go back to throwing PNSE.

@adamsitnik @danmoseley @stephentoub @dotnet/area-system-console wdyt? are there any objections/concerns?

@jeffhandley
Copy link
Member

No objections from me, @tmds. Thanks for digging into this. Feel free to submit a PR that reverts #75824, and we'll mark it as a breaking change.

@jeffhandley jeffhandley added this to the 9.0.0 milestone Mar 21, 2024
@jeffhandley jeffhandley added help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels Mar 21, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Mar 26, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Console blocking-release help wanted [up-for-grabs] Good issue for external contributors in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants