-
-
Notifications
You must be signed in to change notification settings - Fork 805
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
ssh doc addition, and remove compiler warning #6168
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this! I think the scrolling issue is probably more complex, so I'd suggest splitting that out from the other two fixes so that we can get those merged in the meantime
term/src/screen.rs
Outdated
if scroll_region.start != 0 || scroll_region.end as usize != self.physical_rows { | ||
if scroll_region.start != 0 | ||
|| scroll_region.end as usize != self.physical_rows | ||
|| !self.allow_scrollback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this change; it implies that the seqno's of the viewport are not being correctly invalidated by the main logic below for the common case. In particular, I'm concerned that if this is needed, it should probably be unconditional rather than scoped to the !allow_scrollback case.
The reason this is conditional is to avoid an additional O(screen-height) fixed overhead for every scroll operation.
Please translate your repro scenario from #6166 into a unit test in
https://github.com/wez/wezterm/blob/main/term/src/test/mod.rs that triggers the problem; I'd like to see what is happening to the seqno's of the lines being scrolled without attempting to fix it, then we can figure out what the right fix is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
Dropped for now and will resubmit.
For enhancement requested in issue wez#6093
As per <rust-lang/rust#123748> ! will no longer degrades into () which in this situation breaks type deduction; so specify it explicitly.
Thanks! |
Two small separate changes.
Address a compiler warning, and make a small doc change for changes from #6093