Skip to content

Conversation

@rdeline
Copy link
Contributor

@rdeline rdeline commented Jan 11, 2019

This PR introduces a new feature to allow an HTML document to be inset inside a code editor at a given line of code. This feature is exposed to extensions to support a variety of use cases. To illustrate the feature, the PR includes an extension that looks for specially formatted comments in JS/TS files, e.g.
// INLINE some-url
and embeds the contents of that URL directly below the comment line. Another use case is to support a user experience like computations notebooks (e.g. Jupyter) in which script results are shown inlined inside the script code.
This PR is an MVP to illustrate the feature.

@RMacfarlane
Copy link
Contributor

Hi @rdeline, is there an open issue tracking this feature request? Please see our contributing guidelines, we ask that PRs address open issues.

Things like new extension APIs must be discussed with and agreed upon by the feature owner

@rdeline
Copy link
Contributor Author

rdeline commented Jan 12, 2019

Sorry, @RMacfarlane, it's my first PR, so I don't know the conventions. :) Relevant issues include 41775 (which this PR directly addresses), and 34739, and 39492 (which this PR enables, with a further change in the Python extension, which I've also done).

@jrieken
Copy link
Member

jrieken commented Jan 18, 2019

Linking #3220 which I believe was the first request of this kind

@leocb
Copy link

leocb commented Jan 23, 2019

Would this PR allow the HTML/JS to "return" something to the code? example: #66493

@rdeline
Copy link
Contributor Author

rdeline commented Jan 23, 2019

@leocb Yes, it could. The feature is implemented using a webview element, and there is a communication channel between Code and the webview. So, the JS code hosted in the webview can post a message, which the extension can listen for. In the PR, the inline-doc extension uses this approach to communicate the size of the HTML content.

@tecosaur
Copy link

tecosaur commented Jan 27, 2019

@jrieken Looking at the VS Code PR requests, I see some open from 2016, and some merged two days ago. As a result, I thought it was worth asking - is it, and if so when is it, likely that this PR will be merged?

( I come from an extension that is deeply interested in this functionality)

@jrieken
Copy link
Member

jrieken commented Jan 28, 2019

@tecosaur Yes - we do have interest in the PR. No - we don't have a timeline.

@jrieken
Copy link
Member

jrieken commented Jan 28, 2019

@rdeline Thanks so far. I did take this for spin and things look promising. Everything is weird into the right places and it's more about the following items

  • test how well this works in real life as every web view is a separate process.
  • tweak the API

Wrt the API we actually wanna go away from the URL-approach and expose the WebViews instead. Something like this

class CodeInset {
  id: string;
  range: Range;
  height?:number;
}

export interface CodeInsetProvider {
  //...

  resolveCodeInset(inset: CodeInset, webview: Webview, token: CancellationToken): Promise<void>
}

So, the extension provides the insets and the editor calls back with a web view which can be managed by the extension, e.g. webview.html = someHtml. That will also enable the bidi-communication it supports and webview-code will know when a web view is being terminated etc.

@jrieken jrieken added feature-request Request for new features or functionality api api-proposal labels Jan 28, 2019
@rdeline
Copy link
Contributor Author

rdeline commented Feb 2, 2019

@jrieken I like this tweak to the API, particularly since it means that extensions won't have to implement both a CodeInsetProvider and TextDocumentContentProvider to put original HTML in the editor. This suggestion nicely combines these into one class.

I'm trying to update the PR to implement this change, but it's a struggle. The issue is this. The webview needs to be created by MainThreadWebviews.$createWebview in order for an extension to make calls on it. However, the webview also needs to be available to the code inset editor contribution, namely CodeInsetController. So far, I'm not seeing a good way to wire these two components together. Any suggestions would be very welcome. Maybe MainThreadWebviews needs to be a service so that the code inset editor contribution can ask for it in its constructor.

@jrieken jrieken added this to the February 2019 milestone Feb 7, 2019
@rdeline
Copy link
Contributor Author

rdeline commented Feb 7, 2019

I've updated the PR to reflect the requested API change that @jrieken requested. The new API is definitely nicer as can be seen in extensions/inline-doc/extension.ts. I found a way to wire up the pieces that I think is reasonable, but of course all suggestions are welcome.

I think there's an interesting speed/space trade-off with this feature. Webviews both take up memory and are noticeably slow to load. To optimize for space, one could allocate webviews only for the currently visible viewport, but the loading delay would be noticeable to the user. I think it's up to the core Code team to decide an appropriate policy for this trade-off.

@jrieken
Copy link
Member

jrieken commented Feb 11, 2019

@rdeline We have finished our February planning and we wanna go forward with this. The goal is to get this merged quickly and that we take over for the final bits. No promise on when we expose this a real API, for now this will be proposed and we will experiment with the overall performance first. As next steps I would propose

  • merge with master one more time
  • move the sample extension so that it doesn't become part of vs code

@kieferrm kieferrm mentioned this pull request Feb 11, 2019
35 tasks
@gulshan
Copy link

gulshan commented Feb 12, 2019

Can we get a screenshot please?

@jrieken
Copy link
Member

jrieken commented Feb 12, 2019

@rdeline I have squashed and rebased your commits into 98aec46. I have also pushed a few commits which makes code insets a workbench feature - I moved declarations of interfaces and settings. Also, I have pushed 2517974 which undoes some seemingly unrelated changes.

@rdeline
Copy link
Contributor Author

rdeline commented Feb 12, 2019

Screen shot of the sample extension to inline arbitrary URLs inside the code:

screen shot 2019-01-11 at 2 56 11 pm

@PEZ
Copy link
Contributor

PEZ commented Apr 4, 2019

Will it be possible to create floating webviews with this?

@samghelms
Copy link

samghelms commented Apr 4, 2019

@rdeline @jrieken Do you have any examples on how to get this api up and running? I see that there was an example and it looks like it got force push overwritten. It's unclear how to actually trigger the extension after it is registered.

@jrieken
Copy link
Member

jrieken commented Jun 4, 2019

fyi - while tackling some debt I have also jumped on this api proposal and I have changed it to a push model. surely some new regressions have been added, but the API really looks nice:

// #region Joh - code insets
export interface WebviewEditorInset {
readonly editor: TextEditor;
readonly range: Range;
readonly webview: Webview;
readonly onDidDispose: Event<void>;
dispose(): void;
}
export namespace window {
export function createWebviewTextEditorInset(editor: TextEditor, range: Range, options?: WebviewOptions): WebviewEditorInset;
}
//#endregion

@jrieken
Copy link
Member

jrieken commented Jun 4, 2019

sample, implements command that adds an inset:

    vscode.commands.registerCommand('extension.sayHello', async (args, brgs, crgs) => {
        if (!vscode.window.activeTextEditor) {
            return;
        }

        const inset = vscode.window.createWebviewTextEditorInset(
            vscode.window.activeTextEditor,
            vscode.window.activeTextEditor.selection.with({ end: vscode.window.activeTextEditor.selection.end.translate(8) }),
            // { enableScripts: true, enableCommandUris: true }
        );
        inset.onDidDispose(() => {
            console.log('WEBVIEW disposed...');
        });
        inset.webview.html = `<head><meta></head><body><img src="https://imgs.xkcd.com/comics/plutonium.png"/><body>`;
    });

@samghelms
Copy link

@jrieken Is there a way to make the webview html interactive? (So that the user can click on a button in the code inset, for example)

@tamuratak
Copy link
Contributor

@jrieken Great work. The most astonishing point is that this is so fast and lightweight.

I have a question. Insets disappear when we change tabs. Is this an intended behavior?

Jun-08-2019 16-33-58

I am looking forward to this feature coming to the stable branch.

@rdeline
Copy link
Contributor Author

rdeline commented Jun 11, 2019

@jrieken I love the simplified the API, but I'm wondering about this code in the implementation:

		const webview = this._webviewService.createWebview({
			enableFindWidget: false,
			allowSvgs: false,
			extension: undefined
		}, {
				allowScripts: options.enableScripts
			});

As far as I can tell, the fact that extension is null means that the vscode-resource protocol will not be defined properly for the webview, since extension location will be undefined. This means that extensions can't load local resources into inset webviews, which is a real limitation. I think the typical use case is that an extensions will ship with some HTML to be displayed. This is what I'm trying to do in the Python extension, for example.

@jrieken
Copy link
Member

jrieken commented Jun 12, 2019

Insets disappear when we change tabs. Is this an intended behavior

Yes - for the API it means the editor goes aways and comes back (as new editor) and therefore you need to re-add them. This is inline with how decorations work.

As far as I can tell, the fact that extension is null means that the vscode-resource protocol will not be defined properly for the webview,

Ops, that was an oversight and not in the intent. I will push a fix for that

@rdeline
Copy link
Contributor Author

rdeline commented Jun 17, 2019

@jrieken Another piece of feedback on the new API, based on my PR for the Python extension to inline Jupyter results. First, in the new API, the inset range encodes two unrelated pieces of information: the line number where the inset should appear, and the height (in lines) of the inset. The fact that range.end.line - range.start.line encodes the height is unexpected. I think it would be clearer and more useful to provide two separate parameters: createWebviewTextEditorInset(editor, line, height, options).

Second, I think the height needs to be specified in either lines or pixels. Consider the Python case below (still a work in progress). A textual result is easiest to express in lines, but an image result is easiest to express in pixels. Indeed, I cannot find an extension API method to get the line height to do the conversion myself. If such an API call exists, then I suppose it's okay to make the extension writer do the conversion.

image

@jrieken
Copy link
Member

jrieken commented Jun 20, 2019

the fact that range.end.line - range.start.line encodes the height is unexpected.

Took a short cut there... The neat thing with hight in lines (vs pixels) is that is honours different line height settings without further ado. Not yet sure how we could expose that to extension author tho...

jrieken added a commit that referenced this pull request Jun 20, 2019
@jrieken
Copy link
Member

jrieken commented Jun 20, 2019

latest proposal (height is still a multiple of line numbers)

export function createWebviewTextEditorInset(editor: TextEditor, line: number, height: number, options?: WebviewOptions): WebviewEditorInset;

@arash-hacker
Copy link

arash-hacker commented Aug 18, 2019

how access createWebviewTextEditorInset inside vscode .
i update my vscode package but doesn't appear this method?
my vscode.d.ts is not update when install vscode ,@types/vscode and vscode-test
thanks

@dddom
Copy link

dddom commented Oct 1, 2019

Hello, maybe I don't fully understand how this works, but is there a piece of API documentation relevant to this functionality, or is it not fully completed?

@samghelms
Copy link

@melvinroest
Copy link

Wait, I don't get it. I'm really confused.

Can I now use the //INLINE <img_url> command?

(In my code editor it doesn't seem to work)

I rather don't build a Python VSCode extension that is 20+ commits ahead and 300+ commits behind when I'm coding in JavaScript.

@rdeline rdeline deleted the code-inset branch October 15, 2019 17:29
@yozlet
Copy link

yozlet commented Oct 18, 2019

@dddom: The API is still in Proposed status, which means it can't be used in extensions yet, but that's usually a matter of time. You can see the implementation of window.createWebviewTextEditorInset here and what it provides here.

@melvinroest No, the "//INLINE" comment is an example of an extension which could be built to use Code Insets, rather than a description of this feature. So far, there are no user-visible changes to VSCode, and no published extensions can use Code Insets until it leaves Proposed status.

@techsin
Copy link

techsin commented Nov 25, 2019

hi guys complete noob here, is this in production now?

@tamuratak
Copy link
Contributor

FYI: issue #85682 opened.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api api-proposal feature-request Request for new features or functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.