-
-
Notifications
You must be signed in to change notification settings - Fork 512
Update Theia version to 1.25.0 #947
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
Conversation
arduino-ide-extension/src/electron-main/theia/electron-main-application.ts
Outdated
Show resolved
Hide resolved
@msujew CI is failing here, probably because of the number of changes in the yarn.lock
Is it possible to reduce them to a minimum? Probably it'd be enough to unlock the build.
54dff56 to
d36c108
Compare
@AlbyIanna It took me a bit, but it seems to be a bug in node-gyp, which doesn't run correctly on some Windows systems (including GH Actions Windows runners). See this comment. Running npx node-gyp install 14.18.1 during the build seems to fix the incorrect node-gyp setup.
The other updates in the yarn.lock were mandatory, as they were related to updated dependencies used by @theia/* packages.
3ece882 to
ddee44e
Compare
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.
UPDATE: resolved
To reproduce
- Do a minimal simulation of a fresh install by deleting the "User data" folder:
- Windows:
-
%APPDATA%\arduino-ide\
-
- Linux:
-
~/.config/arduino-ide/
-
- macOS:
-
~/Library/Application Support/arduino-ide/
-
- Windows:
- Start the Arduino IDE.
🐛 The IDE hangs forever at the splash screen.
Arduino IDE version
2.0.0-rc5-snapshot-414f51f
(the tester build for 8357946)
Operating system
Windows 10, Ubuntu 20.04
(I didn't try it on macOS.)
Additional context
This part of the command line output looks interesting to me:
root ERROR Failed to start the frontend application.
root ERROR Error: No matching bindings found for serviceIdentifier: Symbol(CompressionToggle)
at file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8809025
at m (file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8809322)
at b (file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8809497)
at file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8810031
at Array.forEach (<anonymous>)
at file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8810010
at Array.forEach (<anonymous>)
at b (file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8809557)
at file:///E:/downloads/arduino-ide_2.0.0-rc5-snapshot-414f51f_Windows_64bit/resources/app/lib/bundle.js:2:8810031
at Array.forEach (<anonymous>)
ddee44e to
8357946
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
No matching bindings found for serviceIdentifier: Symbol(CompressionToggle) error has been resolved. Thanks!
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.
UPDATE: resolved by https://github.com/arduino/arduino-ide/compare/00386d4fddc7a5ce5f4fdc4032096e5b4f1ac67b..a3d38384b93ebeab10ec7f5c402bc859355c261e
To reproduce
- Start the Arduino IDE.
🐛 The splash screen is not shown. - Open an additional window via any available mechanism:
- File > New
- File > Open
- File > Sketchbook
- Sketchbook view
🐛 A splash screen is shown unexpectedly (normally the splash screen is only shown on IDE start up, not when opening an additional window)
🐛 The new window never opens.
🐛 The splash screen never closes.
Arduino IDE version
2.0.0-rc5-snapshot-6269207
(the tester build for 00386d4)
Operating system
Windows 10, Ubuntu 20.04
(I didn't try it on macOS.)
Additional context
The above description is for when only a single window opens on startup of the IDE. The behavior is a little different when multiple windows open on startup of the IDE (due to having been open at the end of a previous session using a working version of the IDE). In this case, one window opens fully and the others do appear but never load fully:
(削除ここまで)(削除) image
(削除ここまで)
A splash screen is shown unexpectedly (normally the splash screen is only shown on IDE start up, not when opening an additional window)
That must be my fault, let me check.
@msujew, could you please drop my suggestion (00386d4). It is causing the issue. Let's put aside that I messed up the condition here 😕, but the logic simply wrong when opening the second, third, etc. windows. We cannot rely on the window tracking from the superclass. I am sorry for the inconvenience, and thanks for the help 🙏
Per, I promise I will do more manual testing before submitting anything.
00386d4 to
a3d3838
Compare
Additional window load failure issue is solved by https://github.com/arduino/arduino-ide/compare/00386d4fddc7a5ce5f4fdc4032096e5b4f1ac67b..a3d38384b93ebeab10ec7f5c402bc859355c261e Thanks!
Per, I promise I will do more manual testing before submitting anything.
Hey now, leave some fun for me Akos! 😆
a3d3838 to
0d7b185
Compare
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.
UPDATE: resolved by https://github.com/arduino/arduino-ide/compare/a70273a6c10d34c6b75c446439da0209c67bfb97..130586e0f40b46d4beef4065fbc9e713ae5d712d
Describe the problem
🐛 The "Auto save" setting in the File > Preferences dialog is always checked, even when you had unchecked it previously.
🐛 The IDE never saves automatically, even though the "Auto save" setting is shown as enabled in the Preferences dialog.
To reproduce
- Select File > Preferences... from the Arduino IDE menus.
- Uncheck the box next to "
☑Auto save". - Click the OK button.
- Select File > Preferences... from the Arduino IDE menus.
🐛 The box next to "☑Auto save" is checked. - Click the OK button.
- Make a change to the sketch.
🐛 ⬤ indicator appears on right side of sketch tab.
🐛 The change is not saved to disk.
Expected behavior
The state of the "Auto save" setting in the File > Preferences dialog reflects my previous selection.
The IDE's auto save behavior reflects the "Auto save" setting's state.
Arduino IDE version
2.0.0-rc6-snapshot-e2a9dd5
(the tester build for 0d7b185)
OS: Windows 10, Ubuntu 20.04
Additional context
This part of the command line output looks interesting:
root WARN Request to validate preference with no type information:
I don't have that when using 2.0.0-rc6
(削除ここまで)
(削除) The problem does not occur when using 2.0.0-rc6
(削除ここまで)
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.
UPDATE: resolved by https://github.com/arduino/arduino-ide/compare/a70273a6c10d34c6b75c446439da0209c67bfb97..130586e0f40b46d4beef4065fbc9e713ae5d712d
The "Go to definition" capability provided by the integrated language server gives the user quick access to the definition of any object referenced in the sketch code.
🐛 Even though the file containing the definition is opened in the IDE, the cursor is not moved to the definition, meaning the user would still need to search through the file to find it.
When the definition is in the currently open file, this means "Go to definition" has no value and no apparent effect.
To reproduce
- Create the following sketch:
void setup() { digitalRead(2); } void loop() {}
- Select Tools > Board > Arduino AVR Boards > Arduino Uno from the Arduino IDE menus.
- Wait for sketch processing to finish.
- Right click
digitalRead.
i There is no special significance of this function other than it provides a more dramatic demonstration due to its definition being located many lines down in the source file. - Select "Go to Definition" from the context menu.
🐛 wiring_digital.c opens as expected, but the cursor is not placed at the location of the digitalRead definition.
Expected behavior
The cursor is placed at the definition of the object that was the target of the "Go to definition" operation.
Arduino IDE version
2.0.0-rc6-snapshot-e2a9dd5
(the tester build for 0d7b185)
OS: Windows 10, Ubuntu 20.04
Additional context
This screencast provides a minimal (but less dramatic) demonstration of the issue. Note that the cursor never moves to line 1 as expected:
Sketch and language server logs from the operation shown in the screencast:
The issue does not occur when using Arduino IDE 2.0.0-rc6
(削除ここまで)
(削除) The issue occurs even when the object reference is in a .cpp or .c source file instead of .ino.
(削除ここまで)
Since I see the same bug in Theia Blueprint 1.24.0, it would likely be considered out of scope for this PR, but maybe worth mentioning that this makes the already buggy "Keyboard Shortcuts" view even worse:
Custom key bindings are never shown in the "Keyboard Shortcuts" view
To reproduce
- Select File > Advanced > Keyboard Shortcuts from the Arduino IDE menus.
- Hover the mouse pointer over "Add Cursor Above".
i there is nothing special about this particular shortcut. The issue occurs with any of them. - Click the pencil icon ("Edit Keybinding").
- Change the value of the field to
alt+ctrl+shift+up - Click the OK button.
🙂 The key binding shown in the "Keybinding" column for "Add Cursor Above" is changed to Ctrl+Alt+Shift+Up as expected. - Select File > New from the Arduino IDE menus.
i the bug affects all windows other than the one the customization was made in, so it will also occur via any operation that opens a new window, including restarting the IDE. - Select File > Advanced > Keyboard Shortcuts from the Arduino IDE menus.
🐛 The key binding shown in the "Keybinding" column for "Add Cursor Above" is the default Ctrl+Alt+Up (even though the custom one is actually in effect rather than the one indicated by the view.
This is the same buggy behavior as in 2.0.0-rc6 - Select the "Search keybindings" field.
- Press any key (e.g., Space
🙁 The The key binding shown in the "Keybinding" column for "Add Cursor Above" is still the default Ctrl+Alt+Up.
In 2.0.0-rc6, the listed key binding finally updates to show the custom setting after you press any key with that field selected.
UPDATE: Still occurs with 2.0.0-rc6-snapshot-fd113af (build for 130586e) and with Theia Blueprint 1.25.0 (Beta)
UPDATE: resolved by https://github.com/arduino/arduino-ide/compare/a70273a6c10d34c6b75c446439da0209c67bfb97..130586e0f40b46d4beef4065fbc9e713ae5d712d
The window no longer automatically reloads after changing the "Language" preference.
I am OK with the new behavior for the following reasons:
- Even with the reload, the locale was not fully updated (other windows and Arduino CLI were not updated)
- The reload could cause loss of user data (Unsaved changes are lost on "Language" preference change #954 )
- The need to reload is clearly communicated to the user
But I thought I should mention it anyway since I think the change was unintended and perhaps it is a symptom of a problem that has other more significant implications.
To reproduce
- Select File > Preferences... from the Arduino IDE menus.
- Select a different language from the "Language" menu.
- Click the OK button.
(削除) 😕 The window does not reload.
(削除ここまで)
@msujew do you have updates on this PR?
@fstasi As noted in Slack, I'm on vacation 'til the 16th of May. Afterwards this'll be my first priority.
I checked and fixed the followings:
"Auto save" preference is not remembered
It should work now. However, there is a drawback. There was a breaking change in Theia and editor.autoSave was renamed to files.autoSave.
Suppose a user disabled the autosave, the "editor.autoSave": "off" was written in the settings JSON. This version ignores the old autosave key/value and starts the IDE2 with autosave enabled. Users can change it.
(Yes, we can read the old vale and make the code backward compatible, but we need additional Theia customization.)
Cursor is not placed at definition by "Go to definition"
I could not reproduce it. Please double-check. Thank you!
go_to_definitions.mp4
The window no longer automatically reloads after changing the "Language" preference.
It's fixed. It reloads. It was a generic reload issue with the first window. I decided to change the splash screen logic once more and clean it up.
Thanks @kittaakos. The build workflow did not run because of the merge conflict. Would you mind resolving that so I can make sure to use a standardized build for the testing?
When reloading the browser window (for example, the language has changed), we show index.html instead of Arduino IDE.
index_html.mp4
Also, when changing the language, we should reload all windows, not just a current one.
The spinner was a change in Theia, but it feels stupid to have the new, fancy spinner, although the IDE startup performance time is the same. Do you want me to restore the old spinner? I vote to restore the old spinner. Thoughts? Thank you!
Dev notes: I merged the main instead of rebasing. Once all the feedback threads are resolved, I will squash rebase.
I found these problems:
Tons of warnings in the logs:
[83713:0519/093905.459621:ERROR:accelerator_util.cc(24)] The accelerator string can only contain ASCII characters
[83713:0519/093905.459641:ERROR:accelerator_util.cc(24)] The accelerator string can only contain ASCII characters
[83713:0519/093905.459661:ERROR:accelerator_util.cc(24)] The accelerator string can only contain ASCII characters
[83713:0519/093905.459684:ERROR:accelerator_util.cc(24)] The accelerator string can only contain ASCII characters
Sometimes the firmware updater gets an incorrect JSON (see here):
root ERROR Request updatableBoards failed with error: Error executing "/Users/a.kitta/dev/git/arduino-ide/arduino-ide-extension/build/arduino-fwuploader" firmware list --format json: unexpected end of JSON input
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x14bd30e]
goroutine 1 [running]:
github.com/arduino/arduino-fwuploader/cli/firmware.list(0x0, 0x0)
/home/runner/work/arduino-fwuploader/arduino-fwuploader/cli/firmware/list.go:66 +0x4e
github.com/arduino/arduino-fwuploader/cli/firmware.newListCommand.func1(0xc000492500, 0xc000158ea0, 0x0, 0x2)
/home/runner/work/arduino-fwuploader/arduino-fwuploader/cli/firmware/list.go:42 +0x39
github.com/spf13/cobra.(*Command).execute(0xc000492500, 0xc000158e80, 0x2, 0x2, 0xc000492500, 0xc000158e80)
/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:856 +0x2c2
github.com/spf13/cobra.(*Command).ExecuteC(0xc0001eb900, 0x0, 0x0, 0x1501260)
/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:960 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:897
main.main()
/home/runner/work/arduino-fwuploader/arduino-fwuploader/main.go:35 +0x3c Error: Error executing "/Users/a.kitta/dev/git/arduino-ide/arduino-ide-extension/build/arduino-fwuploader" firmware list --format json: unexpected end of JSON input
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x14bd30e]
goroutine 1 [running]:
github.com/arduino/arduino-fwuploader/cli/firmware.list(0x0, 0x0)
/home/runner/work/arduino-fwuploader/arduino-fwuploader/cli/firmware/list.go:66 +0x4e
github.com/arduino/arduino-fwuploader/cli/firmware.newListCommand.func1(0xc000492500, 0xc000158ea0, 0x0, 0x2)
/home/runner/work/arduino-fwuploader/arduino-fwuploader/cli/firmware/list.go:42 +0x39
github.com/spf13/cobra.(*Command).execute(0xc000492500, 0xc000158e80, 0x2, 0x2, 0xc000492500, 0xc000158e80)
/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:856 +0x2c2
github.com/spf13/cobra.(*Command).ExecuteC(0xc0001eb900, 0x0, 0x0, 0x1501260)
/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:960 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:897
main.main()
/home/runner/work/arduino-fwuploader/arduino-fwuploader/main.go:35 +0x3c
at ChildProcess.<anonymous> (/Users/a.kitta/dev/git/arduino-ide/arduino-ide-extension/lib/node/exec-util.js:53:31)
at ChildProcess.emit (node:events:394:28)
at Process.ChildProcess._handle.onexit (node:internal/child_process:290:12)
at Process.callbackTrampoline (node:internal/async_hooks:130:17)
Co-authored-by: Mark Sujew <mark.sujew@typefox.io> Co-authored-by: Akos Kitta <a.kitta@arduino.cc> Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
a70273a to
130586e
Compare
Looking good! All the outstanding issues from my previous reviews are resolved except for the keyboard shortcuts UI issue (which I see is also still in Theia Blueprint 1.25.0 (Beta)).
This version ignores the old autosave key/value and starts the IDE2 with autosave enabled. Users can change it.
(Yes, we can read the old vale and make the code backward compatible, but we need additional Theia customization.)
Since the project is still in a prerelease phase, I don't think it is necessary.
I would like to see it be clearly documented in the release notes of the release where this is introduced though.
When reloading the browser window (for example, the language has changed), we show index.html instead of Arduino IDE.
I think it is not a great UX. I believe this change was introduced by 112153f
I would be happy to open an issue about it if you think it is something that should be investigated eventually.
Also, when changing the language, we should reload all windows, not just a current one.
The current limited approach of only reloading the active window was explained in #625:
Known Issues
At the moment only the active window is reloaded. We need to propagate a message to other windows and prompt the user to reload the other windows. Other strategies can better suite our needs, but consider this is a corner case, as usually users change the language just once, and as soon as the install the IDE.
I think @AlbyIanna might have looked into reloading all the windows at one point, but I don't recall the exact details.
Something to consider is that full localization update will also require the preference to take effect in Arduino CLI.
Do you want me to restore the old spinner? I vote to restore the old spinner. Thoughts?
I don't have a preference either way. Maybe @91volt or @gmarchiarduino would be interested in the subject though.
Tons of warnings in the logs:
I haven't been able to reproduce this, even after checking on my Windows and Linux machines.
Do you get it even after clearing your IDE configuration files?
Thank you for the review ❤️ See my comment below. Please let me know if there is anything else I need to do for this PR.
I would like to see it be clearly documented in the release notes of the release where this is introduced though.
I will take care of it.
I would be happy to open an issue about it if you think it is something that should be investigated eventually.
It is worth an issue. It used to work. Now, it's showing index.html.
explained in #625:
Something to consider is that full localization update will also require the preference to take effect in Arduino CLI.
👍 Thank you for linking the PR. If it does not take too much time, I think we need a ticket for this. We can reload all windows, and we need to restart the CLI.
Do you want me to restore the old spinner? I vote to restore the old spinner. Thoughts?
I don't have a preference either way. Maybe @91volt or @gmarchiarduino would be interested in the subject though.
Note: It's already reset, we have the old spinner in the IDE2. +1 for having the old spinner and switching to the new one once the startup is faster. In the long run, we can add a preload. See an example here.
Tons of warnings in the logs:
I fixed this too.
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
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 did some further testing of the build and didn't find any problems. Thanks!
Thank you for the initial changes and the review. 🙏 I am merging the PR.
Motivation
The recent two updates have fixed a lot of issues with Theia, so it would be wise to update.
Closes #852
Change description
Updates the
@theia/*dependencies to1.24.0. Changes any code that is affected by this update. Most changes are due to the monaco uplift and changes in how electron windows are handled/created.Other information
Reviewer checklist