Skip to content

Conversation

@thaJeztah
Copy link
Contributor

It was only used in a single place, and a thin wrapper for ensureInRange().

top := addInRange(window.Top, h.sr.top, window.Top, window.Bottom)
bottom := addInRange(window.Top, h.sr.bottom, window.Top, window.Bottom)
top := ensureInRange(window.Top+h.sr.top, window.Top, window.Bottom)
bottom := ensureInRange(window.Top+h.sr.bottom, window.Top, window.Bottom)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jstarks trying to get my head around if this is correct or a bug; wondering if this should be;

Suggested change
bottom := ensureInRange(window.Top+h.sr.bottom, window.Top, window.Bottom)
bottom := ensureInRange(window.Bottom+h.sr.bottom, window.Top, window.Bottom)

Looks like it was added in 396f10f

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, I believe it's correct as-is. The effective scroll region is relative to the window rectangle, so adding to the window top is the right thing.

Of course I wrote this code 5 years ago so who knows ;)

It was only used in a single place, and a thin wrapper for
ensureInRange().

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the remove_addInRange branch from de1cfac to c774e9b Compare July 3, 2023 14:33
@thaJeztah
Copy link
Contributor Author

rebased

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