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

feat: support custom handler for "open in editor" #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

antfu
Copy link

@antfu antfu commented May 21, 2024

Thank you for this awesome project! It's really helpful!

The only thing I found a bit tedious was locating the exact line of the functions that we were inspecting. Having a "open in editor" feature would be greatly helpful to the overall workflow.

While this could be a bit tricky to do due to the the web being by nature sandboxed, and depending on what context it runs, it might have a different approach to link to the local file. To make it easy to extend and suitable for different scenarios, I think providing an injectable "handler" function would be the best fit, while later we could figure out a better default handling.

I built a VS Code extension for open .cpuprofile with cpupro, and I want it to be able to redirect me directly to the code. I used a local patched version of cpupro, and here is what I have:

Screen.Recording.2024-05-21.at.10.31.41.3.mp4

And here is how I am doing it: https://github.com/antfu/vscode-cpupro/blob/798dcc5a6f07128a420a5caeeec9c7322fd9a261/src/index.ts#L54-L63

(off topic, that I also made https://github.com/localfile-link/localfile-link to allow opening editor from any website, via a client app handle the custom protocol, similar to how Zoom link works - if you are interested in it, I'd be happy to discuss about how we could integrate that for the web version)

@lahmatiy
Copy link
Member

Hey @antfu, thank you for your contribution!

It's great to see cpupro being integrated into VS Code. I'd like to propose an alternative approach leveraging Discovery.js, on which cpupro is built. Discovery.js includes an Embed API that allows for communication with sandboxed apps. Although I haven't tested this in the VS Code environment, I'm believe that we can address potential issues if any.

The Embed API enables an embedder to specify actions for the app, which can be executed based on user interactions. This setup ensures that the app can offer additional features when an action is defined, without knowing the specifics of the action's implementation and eliminating the need for "magic" URLs. In VS Code, these actions could be executed according to the extension's permissions.

Here's a simple implementation example for an HTML page:

<iframe id="cpupro-iframe" src="file://path/to/cpupro.html"></iframe>
<script type="module">
    import { connectToEmbedApp } from "@discoveryjs/discovery/dist/discovery-embed.js";
    
    const disconnect = connectToEmbedApp(document.getElementById('cpupro-iframe'), (app) => {
        app.defineAction('openFile', (filename) => {
           // do something with filename
        });
    });
</script>

In the app:

{
    view: 'link',
    when: '#.actions.openFile',
    onClick: '=#.actions.openFile',
    content: 'text:"Open in editor"'
}

The required changes for the loc-badge:

discovery.view.define('loc-badge', {
    view: 'badge',
    className: 'function-loc',
    data: 'function or $ | marker("function").object |? { ..., text: loc }',
+   onClick: '"openFile".actionHandler(`${module.path}${loc}`)',
    whenData: 'text',
    content: 'html:text.split(/:/).join(`<span class="delim">:</span>`)'
}, { tag: false });

However, it appears that the onClick event handler is not currently supported for the badge component, which seems to be an oversight. I plan to address and resolve this in Discovery.js asap. For now, you may use a link as a temporary workaround:

discovery.view.define('loc-badge', {
-   view: 'badge',
+   view: 'link',
-   className: 'function-loc',
+   className: 'view-badge function-loc',
    data: 'function or $ | marker("function").object |? { ..., text: loc }',
+   onClick: '"openFile".actionHandler(`${module.path}${loc}`)',
    whenData: 'text',
    content: 'html:text.split(/:/).join(`<span class="delim">:</span>`)'
}, { tag: false });

To define an action directly in the app (for testing purposes) use discovery.action.define:

discovery.action.define('openFile', filename => alert(filename));

@antfu
Copy link
Author

antfu commented May 21, 2024

Thanks for the detailed guides, that look great!

While I do not have the page in an iframe, is it possible to get the discovery from an append script added to report.html? Maybe exposing it to window.discovery similar to discoveryLoader?

@lahmatiy
Copy link
Member

Maybe exposing it to window.discovery similar to discoveryLoader?

discoveryLoader is necessary for one specific scenario (loading data from the HTML file itself). window.discovery also works only for a limited number of cases. In fact, there can be several Discovery.js apps on the page as widgets; in this case, window.discovery will point to the last instance. Besides, leaking to the global scope isn't a good practice at all.

You can create an <iframe> in your generated code and make a bridge between cpupro in the <iframe> and the VS Code API. It's not necessary to include HTML inline. See this example on how to inject URLs as resources in the VS Code environment.

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