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

Add region-scrolling commands. #918

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nfachan
Copy link

@nfachan nfachan commented Aug 24, 2024

For terminals that support ANSI, these are implemented by temporarily setting the scrolling region, scrolling, then resetting the scrolling region.

For now, these are not implemented for non-ANSI windows terminals.

I don't think I need to implement is_ansi_code_supported, since it seems like the default implementation should be good. These ANSI escape sequences are supported by Windows.

I plan on following up with a windows implementation eventually.

For terminals that support ANSI, these are implemented by temporarily
setting the scrolling region, scrolling, then resetting the scrolling
region.

For now, these are not implemented for non-ANSI windows terminals.
@nfachan nfachan requested a review from TimonPost as a code owner August 24, 2024 01:49
@joshka
Copy link
Collaborator

joshka commented Aug 24, 2024

The rationale for splitting these into multiple commands would be that in an app you might have a scroll region, which you set once at the start of the app, scroll it whenever you need to, and reset it at the end of the app. That is to say, I liked the previous PR where this was broken out. This seems like it's possibly one layer of abstraction above where most of crossterm's commands are targeted. Do you agree, or have I missed something?

@nfachan
Copy link
Author

nfachan commented Aug 26, 2024

The rationale for splitting these into multiple commands would be that in an app you might have a scroll region, which you set once at the start of the app, scroll it whenever you need to, and reset it at the end of the app. That is to say, I liked the previous PR where this was broken out. This seems like it's possibly one layer of abstraction above where most of crossterm's commands are targeted. Do you agree, or have I missed something?

I guess it depends on how you want to support the old windows API. If the plan is to never support it, then I think hewing close to the ANSI escape sequences is best (i.e. my original PR where there were just commands for setting and resetting the scrolling region). On the other hand, if you do want to support the old windows API, you'd need something like this that's implemented at a bit of a higher level.

I personally don't have a preference. I just want to do whatever needs to be done to get ratatui to support scrolling regions by default!

@nfachan
Copy link
Author

nfachan commented Aug 27, 2024

I created #923 as an alternative.

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