-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Support link position in hover tooltip callback #2470
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
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 @jmbockhorst, I haven't tested it but this looks good. We mainly just need to figure out what naming we want to commit to
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.
@jmbockhorst let's change the API to use this instead:
interface IViewportRange {
start: IViewportCellPosition;
end: IViewportCellPosition;
}
interface IViewportCellPosition {
col: number;
row: number;
}
I created #2480 to make ISelectionPosition more consistent for v5
matcher.hoverTooltipCallback(e, uri); | ||
// Note that IViewportRange use 1-based coordinates to align with escape sequences such | ||
// as CUP which use 1,1 as the default for row/col | ||
matcher.hoverTooltipCallback(e, uri, { start: { row: y1 + 1, col: x1 + 1 }, end: { row: y2 + 1, col: x2 } }); |
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 made a change to expose this as a 1-based number to align with https://invisible-island.net/xterm/ctlseqs/ctlseqs.html. It's looks a little weird how the end col doesn't add 1 but I guess that's how the mouse zones work right now, they include everything up to but excluding the end cell
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.
Did you forget col: x2
here? Shouldn't it be col: x2 + 1
as like the others?
@mofux the right numbers are coming through like that, I think mouse zone uses coords a little strangely (I wrote it 😅) and the end isn't included in the range |
Added support for link position in the hover tooltip callback and tested that it works in the demo.
I defined
ILinkLocation
in both the public API and the browser types. I know that this probably isn't correct, but I wasn't sure how to get around it. Closes #2468.