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

Use 'openWithSystemApp' to open uri when 'env.openExternal' requested #13676

Merged

Conversation

dhuebner
Copy link
Contributor

@dhuebner dhuebner commented May 3, 2024

What it does

When vscode.env.openExternal is requested, opens the URI using window.electronTheiaCore.openWithSystemApp instead of opening the file in the default Theia editor.

See #13535

How to test

Follow the "Steps to Reproduce" instructions in #13535

Follow-ups

Review checklist

Reminder for reviewers

@dhuebner dhuebner force-pushed the dhuebner/externalAppOpener-13535 branch from e0061c5 to ac9b2c4 Compare May 8, 2024 11:45
@dhuebner dhuebner force-pushed the dhuebner/externalAppOpener-13535 branch from ac9b2c4 to 1eb22ea Compare May 8, 2024 12:07
@dhuebner dhuebner marked this pull request as ready for review May 8, 2024 12:14
Copy link
Contributor

@rschnekenbu rschnekenbu left a comment

Choose a reason for hiding this comment

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

I tested using this extension:

That fix works in Theia (electron of course). Code seems good also for me.

@rschnekenbu
Copy link
Contributor

Note: I tested on a rebase of the current branch on master, to check on latest master. It took me a long time to be able to review this one :/

@dhuebner
Copy link
Contributor Author

@rschnekenbu
Thank you very much for the re-view!

@msujew msujew merged commit 40adb22 into eclipse-theia:master Jun 25, 2024
14 checks passed
@jfaltermeier jfaltermeier added this to the 1.51.0 milestone Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants