Skip to content

Conversation

@patrickbkr
Copy link
Contributor

This makes this dist work on Windows and also replaces the VT based terminal size detection.

Part of a set of PRs in Terminal-LineEditor, Terminal-Print and Terminal-Widgets.

Don't merge yet, Terminal-API isn't released yet!

@patrickbkr
Copy link
Contributor Author

@patrickbkr
Copy link
Contributor Author

I'd like to get feedback on the PRs first and when things look good release Terminal-API. This will allow me to still make changes to T-API without burning version numbers.

$p
my $fd = $.output.native-descriptor;
my $size = Terminal::API::get-window-size($fd);
Promise.kept(($size.rows, $size.cols))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about the reason for this change ... does the clipping behavior trick not work on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't try, but would suspect that it'd work. I seem to recall that you explicitly asked for an API to retrieve the window size, I think the cause was Terminal-Print, where the size was retrieved via tput cols/rows. The change here was just for uniformness / simplicity. Would you prefer to keep it as it was before?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's separate out this bit into a separate PR. We may need it, but I'd rather not make this change as part of this PR. (Though both small, the two diff blocks are in my mind two separate conceptual changes, one of which may or may not be needed.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

This makes this dist work on Windows.
@japhb
Copy link
Owner

japhb commented May 13, 2025

Will merge this one and then bump the Terminal::API requirement to latest as of this writing (1.0.3).

@japhb japhb merged commit 6cbd8fc into japhb:main May 13, 2025
0 of 2 checks passed
@patrickbkr patrickbkr deleted the terminal-api branch July 21, 2025 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants