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

terminal: set PageList viewport to active area when cloned #1596

Merged

Conversation

gpanders
Copy link
Member

@gpanders gpanders commented Mar 18, 2024

As an optimization, the renderer does not attempt to find the cell under
the cursor if the viewport is in the scrollback (i.e. not the active
area). When the renderer clones the screen state it also clones the
PageList, and the cloned PageList has its viewport set to the top of the
scrollback.

This caused the renderer to never attempt to find the cell under the
cursor, which in turn caused cells under the cursor to be improperly
highlighted. Instead, when the PageList is cloned initialize its
viewport to the active area.

Fixes: #1584 (comment)

As an optimization, the renderer does not attempt to find the cell under
the cursor if the viewport is in the scrollback (i.e. not the active
area). When the renderer clones the screen state it also clones the
PageList, and the cloned PageList has its viewport set to the top of the
scrollback.

This caused the renderer to never attempt to find the cell under the
cursor, which in turn caused cells under the cursor to be improperly
highlighted. Instead, when the PageList is cloned initialize its
viewport to the active area.
@mitchellh mitchellh merged commit cae1f0f into ghostty-org:paged-terminal Mar 19, 2024
14 of 16 checks passed
@mitchellh mitchellh deleted the page-list-clone-viewport branch March 19, 2024 04:04
@mitchellh
Copy link
Contributor

Thanks! Yeah, I think this is right. I think before we didn't ensure we had enough rows but looking at the code now we do ensure this so I think this is good.

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