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

Get position #281

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Get position #281

wants to merge 2 commits into from

Conversation

javier-m
Copy link

Added the widget argument (containing line and column) to the callback

@@ -108,6 +110,10 @@ export class InfoProvider implements Disposable {
}
}

onNewTacticState(handler_name: string, handler: (state: string, widget: object) => void): void {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onNewTacticState(handler_name: string, handler: (state: string, widget: object) => void): void {
onNewTacticState(handler_name: string, handler: (loc: Location, state: InfoResponse) => void): void {

The widget field doesn't really contain any information (it's an id that you pass to the get_widget command). You probably also want the location (filename & position) so that you can insert something there.

Can you explain what the handler_name should be? Is this the name of the other extension?

I'm not sure if it's possible, but the nicest way would be to use the EventEmitter class from vscode:

    private onNewTacticStateEmitter = new EventEmitter<{ location: Location, info: InfoResponse }>();
    onNewTacticState = this.onNewTacticStateEmitter.event;

@@ -128,4 +129,6 @@ export async function activate(context: ExtensionContext): Promise<void> {

context.subscriptions.push(languages.registerDocumentLinkProvider(LEAN_MODE,
new LibraryNoteLinkProvider()));

return {'infoView': infoView}; // export infoView for other plugins to use
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return {'infoView': infoView}; // export infoView for other plugins to use
return {onNewTacticState: infoView.onNewTacticState}; // export infoView for other plugins to use

I'd rather export only the event.

@@ -120,12 +126,17 @@ export class InfoProvider implements Disposable {
payload: JSON.stringify(e)
})
),
this.proxyConnection.jsonMessage.on(e =>
this.proxyConnection.jsonMessage.on(e => {
if ('record' in e && 'state' in e.record && 'widget' in e.record){
Copy link
Member

Choose a reason for hiding this comment

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

This will trigger on every hover as well (i.e., when you move the mouse around). A better place to intercept would be here:


You'd then need to send a message from the webview back to the extension like this:
export function copyText(text: string): void {
post({ command: 'copy_text', text});
}

And receive the message here:
case 'copy_text':

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