-
Notifications
You must be signed in to change notification settings - Fork 131
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
Theia 1.53.0 #384
Theia 1.53.0 #384
Conversation
d77fe6c
to
b341482
Compare
For testing the theia scheme on ubuntu I slightly adjusted the generated desktop file:
The referenced launcher script looks like this (basically adding logging of the passed arguments):
Now I can see that the AppImage gets the theia scheme URL passed:
Nothing happens though (besides that a new frontend is started), but I guess this is then an issue with plain Theia and not with the IDE and the metadata in the desktopfile |
* create desktop file for linux desktop integration * add protocol for mac Contributed on behalf of STMicroelectronics
b341482
to
fd05efb
Compare
On Windows and Mac the |
I can see that VSCode uses two desktop files, one passing Edit: That seems to do the trick, I'll adapt accordingly |
* add dedicated URL handler desktop file as well * remove MimeType from builder config again, since we need additional open-url args
@@ -19,7 +19,7 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
os: [windows-2019, ubuntu-latest, macos-14] | |||
os: [windows-2019, ubuntu-latest, macos-12] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the downgrade to 12?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the removal of macOS-11, we haven't had a working verification build for macOS, except on the Eclipse CI. See https://github.com/eclipse-theia/theia-blueprint/actions/workflows/build.yml . I started addressing this issue by aligning our approach with that of the CDT Cloud Blueprint, as seen here: eclipse-cdt-cloud/cdt-cloud-blueprint@fddcf9a
Generally, I believe it's better to use older machines for verification jobs because if something works on macOS 12, it's likely to work on macOS 14 as well. However, if it only runs on macOS 14, we cannot be sure it will work on macOS 12.
@@ -1,5 +1,5 @@ | |||
/******************************************************************************** | |||
* Copyright (C) 2022 EclipseSource and others. | |||
* Copyright (C) 2022-2024 EclipseSource and others. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we update the headers for files we change, usually. Not sure if there is a policy and what it is.
} | ||
|
||
@injectable() | ||
export class TheiaDesktopFileServiceEndpoint implements BackendApplicationContribution { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this have to be a separate endpoint? Can't we just make it a back-end service?
Also: can we scope this to linux?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to be consistent with the existing code in that package that may create a launcher script that is available on the path. If we refactor this, then we should probably update both.
@@ -0,0 +1,151 @@ | |||
/******************************************************************************** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my reading on the internet, I was under the impression that electron-builder generates a .desktop file from the info in electron-builder.yml automatically. Am I mistaken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can. The AppImage then does include a desktop file, but it's packaged inside the AppImage. Since the AppImage is an executable, users who directly execute it won't see the desktop file unpacked though. Typically, an AppImage launcher is needed to unpack the desktop file, but not all users install such a launcher as it's often not necessary.
Additionally, the generated desktop file behaves as described in my earlier comment: Issue Comment Link. It simply opens a new Theia window without invoking the URL.
For having a regular launcher and the url handler, I think we need to have two desktop files, as VS code does as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"...and this my friend, is why 2024 will not be the year of Linux on the desktop" ;-)
What it does
theia://
links. #378 on windows)theia://
links. #378 on linux)update-desktop-database ~/.local/share/applications/
after the desktop file was createdtheia://
links. #378 on mac)closes #378
closes #383
Contributed on behalf of STMicroelectronics
How to test
theia://
protocol handling (see Implement previously stubbed window#registerURIhandler (#13169) theia#13306 for instructions)~/.theia-ide/globalStorage/theia-ide-launcher/desktopfile.json
Review checklist
Reminder for reviewers