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

Allow opening changes for files associated with custom editors #13916

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

Conversation

pisv
Copy link
Contributor

@pisv pisv commented Jul 11, 2024

What it does

Allows the user to visually compare two versions of a file that is associated with a custom editor. This is done by opening both versions of the file simultaneously in a side-by-side editor, with each version being opened in a separate custom editor contained in the side-by-side editor.

For example, this feature can be used to help spot the differences between two versions of an image file associated with the custom editor contributed by the VS Code media-preview plugin, as illustrated with the following screenshot:

Screenshot

More screenshots illustrating this feature being used with other custom editors:

Screenshot Screenshot

This PR contains a series of individual commits that work together to make it possible for the CustomEditorOpener to open a diff-uri in a side-by-side editor containing the corresponding CustomEditors. While the main functionality is contributed by the latest commit, the earlier commits provide various fixes and enhancements required to make it work.

Here is the sequence of commits:

  • Fix loading of webview resources that depend on query params

    Some resource url (notably git) use the query part of the url to store additional information. The query part of the url was incorrectly being dropped while attempting to load these resources inside of webviews.

  • Fix Error: Unknown Webview messages in the log

  • Fix design and implementation issues surrounding CustomEditorOpener

    This commit ensures that the promise returned by CustomEditorOpener.open will only resolve to a properly initialized and opened CustomEditorWidget. In particular, it ensures that the widget is opened according to the specified WidgetOpenerOptions, including widgetOptions.ref and mode.

    Essentially, it revises the work done in Fix #9670: Custom editor plugin activation called 3 times when double click quickly #9671 and Fix the widget options when opening a VSCode Custom Editor #10580.

  • Restore custom editors as part of layout

    Fixes an incorrect assumption that a custom editor cannot be restored if no WebviewPanelSerializer is registered for its view type. (Actually, custom editors are created and restored using a custom editor provider.)

    Also, ensures that CustomEditorWidget.modelRef satisfies the shape for the CustomEditorWidget defined in editor.ts and cannot return undefined. (However, CustomEditorWidget.modelRef.object can be undefined until the custom editor is resolved.)

    Fixes custom-editor: restore editor as part of layout #10787

  • Fix a race condition when file system provider is activated

    When file system provider is activated, wait until it is registered.

  • git: add support for custom editors

    • Uses OpenerService instead of EditorManager to open editors

    • Contributes a FileSystemProvider for git-resources

    • Fixes an issue with getting blob contents

  • custom editor: open a diff-uri in a side-by-side editor

    CustomEditorOpener is now able to open a diff-uri in a side-by-side editor, which contains the corresponding CustomEditors.

    Fixes [Diff] Images should open in preview #9079

How to test

There are various configurations that can be used for testing this feature:

  • Windows vs. Linux vs. macOS

  • Browser vs. Electron

  • vscode.git vs. (now deprecated) @theia/git

This feature can be tested with any custom editor.

A basic test scenario involves making a change to a file associated with a custom editor (e.g., .jpg, .png, .pawDraw, or .cscratch) and then opening changes in a side-by-side editor by clicking on the file in the Source Control view.

Since this PR also fixes an issue with custom editors not being restored as part of layout, it is worth checking out that custom editors are now correctly restored, both standalone and as part of side-by-side editors.

Also, it is important to verify that there are no regressions, especially that standalone custom editors can still be opened and work as expected.

Review checklist

Reminder for reviewers

@pisv
Copy link
Contributor Author

pisv commented Jul 11, 2024

All checks are now green, and this PR is eagerly waiting for a review :-)

@msujew
Copy link
Member

msujew commented Jul 23, 2024

FYI, I'll likely take a look at this, but don't have time for it before the upcoming release. Probably next week.

@msujew msujew self-requested a review July 23, 2024 15:28
@msujew msujew added diff editor issues related to the diff editor custom-editor issues related to custom-editor functionality labels Jul 23, 2024
@pisv
Copy link
Contributor Author

pisv commented Jul 23, 2024

@msujew Good to know! Thank you. I'd really appreciate your review.

@msujew
Copy link
Member

msujew commented Aug 7, 2024

Sorry for the delay, but unexpected deadlines came up. Will have time to review starting next week :)

@pisv
Copy link
Contributor Author

pisv commented Aug 7, 2024

No worries :) That'd be great! Thanks!

Some resource url (notably git) use the query part of the url to store
additional information. The query part of the url was incorrectly being
dropped while attempting to load these resources inside of webviews.

See also microsoft/vscode@48387df
This commit ensures that the promise returned by `CustomEditorOpener.open`
will only resolve to a properly initialized and opened `CustomEditorWidget`.
In particular, it ensures that the widget is opened according to the specified
`WidgetOpenerOptions`, including `widgetOptions.ref` and `mode`.

Essentially, it revises the work done in eclipse-theia#9671 and eclipse-theia#10580
to fix eclipse-theia#9670 and eclipse-theia#10583.
Fixes an incorrect assumption that a custom editor cannot be restored
if no `WebviewPanelSerializer` is registered for its view type. (Actually,
custom editors are created and restored using a custom editor provider.)

Also, ensures that `CustomEditorWidget.modelRef` satisfies the shape for the
`CustomEditorWidget` defined in `editor.ts` and cannot return `undefined`.
(However, `CustomEditorWidget.modelRef.object` can be `undefined`
until the custom editor is resolved.)

Fixes eclipse-theia#10787
When file system provider is activated, wait until it is registered.
* Uses `OpenerService` instead of `EditorManager` to open editors

* Contributes a `FileSystemProvider` for git-resources

* Fixes an issue with getting blob contents
`CustomEditorOpener` is now able to open a diff-uri in a side-by-side editor,
which contains the corresponding `CustomEditor`s.

Fixes eclipse-theia#9079
@pisv
Copy link
Contributor Author

pisv commented Aug 7, 2024

(I've resolved a merge conflict and retested everything in preparation to the upcoming review.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
custom-editor issues related to custom-editor functionality diff editor issues related to the diff editor
Projects
Status: Waiting on reviewers
Development

Successfully merging this pull request may close these issues.

custom-editor: restore editor as part of layout [Diff] Images should open in preview
2 participants