-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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 editorLineNumber
context key
#176659
Add editorLineNumber
context key
#176659
Conversation
return; | ||
} | ||
|
||
const anchor = { x: e.event.posx, y: e.event.posy }; | ||
const lineNumber = e.target.position.lineNumber; | ||
|
||
const contextKeyService = this.contextKeyService.createOverlay([['editorLineNumber', lineNumber]]); |
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 think you need to dispose this after you close the menu.
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 was wondering about that, but every other consumer of createOverlay
doesn't dispose it.
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.
Maybe worth an issue for the other cases then--and I'll update this PR to dispose it.
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.
It looks like dispose no-ops on it, so probably okay.
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 think the better thing would splitting out the IContextKeyService interface so that .dispose() doesn't have to be defined on something that doesn't need to be disposed.
Re: #175945