-
-
Notifications
You must be signed in to change notification settings - Fork 395
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
chore: Theia 1.37.0 #2027
chore: Theia 1.37.0 #2027
Conversation
Upstream: sindresorhus/cpy#109 Update: I patched it in IDE2, and provided a fix: sindresorhus/cpy#110. |
c155a33
to
b223ec3
Compare
b223ec3
to
d66da25
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: fixed!
The default value of the workbench.iconTheme
setting changed from none
to theia-file-icons
.
I don't have a strong opinion on the subject of what the default should be. However, the decision the last time the subject was discussed ((#1173) was that the default should be none
and I am not aware of any change to the decision since then so I assume this was an inadvertent change.
To reproduce
- Select File > Quit from the Arduino IDE menus if it is running.
- Delete the file at the following path:
- Windows:
(where
C:\Users\<username>\.arduinoIDE\settings.json
<username>
is your Windows username) - Linux:
~/.arduinoIDE/settings.json
- macOS:
~/.arduinoIDE/settings.json
- Windows:
- Start Arduino IDE.
🐛 There is an icon on the left side of the editor tabs:
- Open the bottom panel.
🐛 There is an icon on the left side of the bottom panel tabs:
- Run the "Preferences: Open Settings (UI)" command.
- Search for the
workbench.iconTheme
setting.
🐛 workbench.iconTheme
is set to "File Icons (Theia)".
Expected behavior
The default value of the workbench.iconTheme
setting is none
.
Arduino IDE version
fc4882c (tester build for d66da25)
Operating system
Windows 11
Thanks for noticing it. An upstream change (eclipse-theia/theia@b33ab2c) broke the default behavior. I've adjusted it here: 7ff070c. |
arduino-ide-extension/src/browser/contributions/startup-tasks-executor.ts
Show resolved
Hide resolved
arduino-ide-extension/src/electron-browser/electron-app-service.ts
Outdated
Show resolved
Hide resolved
arduino-ide-extension/src/browser/contributions/startup-tasks-executor.ts
Outdated
Show resolved
Hide resolved
f2f347a
to
21f3858
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Thanks for checking it. The breaking change was eclipse-theia/theia@1b5ff9e#diff-d8d45a890507a01141c010ad4a6891edf2ae727cfa6dfe604cebbd667812cccbR68. I fixed it |
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: Fixed by 6adde6b
Describe the problem
🐛 Selecting the Edit > Paste menu item does not have any effect.
To reproduce
- Select File > New Sketch from the Arduino IDE menus.
- Select the text
// put your setup
- Select Edit > Copy from the Arduino IDE menus.
- Move the cursor to another location in the sketch.
- Select Edit > Paste from the Arduino IDE menus.
🐛 The copied text is not pasted.
The IDE displays a warning notification:
⚠ Please use the browser's paste command or shortcut.
Expected behavior
Select Edit > Paste from the Arduino IDE menus pastes the content of the clipboard into the editor at the cursor location.
Arduino IDE version
0f1bf56 (tester build for 45aeb7f)
Operating system
- Windows 11
- Ubuntu 22.04
Additional context
The fault does not occur when using the build from the main
branch (51f69f6)
I am not able to reproduce the fault on my macOS machine.
The fault also occurs when using the "Paste" item of the editor context menu.
Pasting via the Ctrl+V keyboard shortcut works as expected, so the fault is specific to the Edit > Paste menu item.
I notice no keyboard shortcut is listed for Paste in the Edit menu:
compare to 51f69f6:
The shortcut is also not defined for the "Paste" command in the "Keyboard Shortcuts" view:
compare to 51f69f6:
However, the fault still occurs even after I assign the Ctrl+V keybinding to the "Paste" command.
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: fixed by 6adde6b
Describe the problem
The sketch folder is not opened when you select Sketch > Show Sketch Folder from the Arduino IDE menus.
To reproduce
- Select Sketch > Show Sketch Folder from the Arduino IDE menus
🐛 The folder of the sketch is not opened in the file browser.
Expected behavior
The folder of the sketch currently open in the IDE window is opened in the file browser when Sketch > Show Sketch Folder is selected from the Arduino IDE menus.
Arduino IDE version
0f1bf56 (tester build for 45aeb7f)
Operating system
Windows 11
Additional context
The fault does not occur when using the build from the main
branch (51f69f6)
The fault does not occur on my Linux and macOS machines.
Nothing is printed to the logs when I do a Show Sketch Folder operation.
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 7534d8a
This is an expansion of the existing problem tracked at #1394
Maybe the conclusion will be the same:
The LS should use them. Instead of removing the context menu items
but I thought it was at least worth mentioning because we now have a significant number of non-functional items and I am not aware of any work, plans, or investigation related to bringing them into a functional state.
Describe the problem
New items were added to the editor context menu:
- Go to Declaration
- Peek > Peek Declaration
- Go to Type Definition
- Peek > Peek Type Definition
- Go to Implementations
- Peek > Peek Implementations
- Go to References
- Peek > Peek References
🐛 None of the newly added items are functional.
To reproduce
- Create a sketch with the following content:
typedef int foo_t; foo_t bar(); foo_t bar() { return 42; } void setup() { bar(); }
- Select Tools > Board > Arduino AVR Boards > Arduino Uno from the Arduino IDE menus.
- Wait for the the "indexing" process to finish, as indicated at the left side of the status bar.
- Right click on the
bar
on line 7. - Select "Go to Declaration" from the context menu.
🐛 Nothing happens (expected result is that the cursor would move to thebar
function declaration at line 2) - Right click on the
bar
on line 7. - Select "Peek > Peek Declaration" from the context menu.
🐛 Nothing happens (expected result is that a peek of thebar
declaration would open) - Right click on the
foo_t
on line 2. - Select "Go to Type Definition" from the context menu.
🐛 Nothing happens (expected result is that the cursor would move to thefoo_t
type definition at line 1) - Right click on the
foo_t
on line 2. - Select "Peek > Peek Type Definition" from the context menu.
🐛 Nothing happens (expected result is that a peek of thefoo_t
type definition would open) - Right click on the
bar
on line 2. - Select "Go to References" from the context menu.
🐛 Nothing happens (expected result is that a peek would open with all references tobar
) - Right click on the
bar
on line 2. - Select "Peek > Peek References" from the context menu.
🐛 Nothing happens (expected result is that a peek would open with all references tobar
)
Expected behavior
All editor context menu items are functional.
Arduino IDE version
0f1bf56 (tester build for 45aeb7f)
Operating system
Windows 11
Additional context
The non-functional menu items are not present when using the build from the main
branch (51f69f6)
I did not test "Go to Implementations" and "Peek Implementations" because I don't know what "implementation" means in the context of a C++ program.
We must update the minimum Node.js version. It's
Update: It's
|
Upstream: eclipse-theia/theia#12487 I will investigate. Thank you for noticing it 🙏 Update: I found a quick workaround: 6adde6b |
arduino-ide-extension/src/browser/dialogs/ide-updater/ide-updater-component.tsx
Show resolved
Hide resolved
Done. It's updated to 16.14.0.
I have fixed it and verified it from the latest build. Please re-check
I added a workaround. It worked for me with the latest build on Windows. My build: Version: 2.1.1-snapshot-e2539a8 Copyright © 2023 Arduino SA |
This comment was marked as resolved.
This comment was marked as resolved.
Very nice 👍 I have updated the VS Code extension language pack versions to match Theia's supported VS Code API version: https://github.com/eclipse-theia/theia/blob/v1.37.0/dev-packages/application-package/src/api.ts#L21. See the changes: I also provided a better workaround for the cut/copy/paste. I will wait for a Windows build and do a double check, but it works on macOS as it was with 2.1.0 |
Some supported languages do not have a more recent extension version available. We must check why and what are the implications of using older language packs with Theia using VS Code 1.72.x APIx:
|
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 bcb7f7d
After opening an IDE window, some sort of an initialization process continues even after the UI appears in the window.
🐛 Some actions have no effect if the user performs them during this time window.
To reproduce
- Select File > New Sketch from the Arduino IDE menus.
- Wait until the UI populates in the new window.
- Immediately* use the mouse to select File > New Sketch from the Arduino IDE menus.
On Windows, this must be done while the mouse pointer has the "working in background" style while hovered over the IDE window:
🐛 A new window does not open.
Expected behavior
UI elements are usable whenever they are exposed to the user.
Arduino IDE version
0fb9a06 (tester build for ed60048)
Operating system
- Windows
- macOS
Operating system version
- Windows 11
- macOS Ventura
Additional context
The fault does not occur when using the build from the main
branch (964ea3b)
I used New Sketch in the demo, but the fault occurs with other UI operations as well.
I was not able to reproduce the fault on my Linux machine.
The fault does not occur when a keyboard shortcut (e.g., Ctrl+N) or even keyboard navigation of the GUI is used to trigger the operation instead of the mouse.
The fault does not occur if I make a keystroke before the UI action.
* On my high spec PC the window is ~5 seconds. It will likely be significantly longer for users with lower performance systems (and thus those users will be statistically more likely to be impacted by the regression).
Something needs to be corrected. We might need to pick another version like we did last time. Related: #1449 |
Fixed. Also, #1394 should be fixed. Please mention in the review if something else needs to be removed. Thanks!
Switched to 1.17.0 language extension.
The ugliest workaround is available for verification, but it works. Please verify it. hack_for_nonfunctional_menu_items.mp4Please revisit the behavior with the most recent changes. Thanks a lot for your persistence. This PR takes way longer than I anticipated. |
This still applies 👇 . From #2027 (comment):
|
The issues I reported via my previous reviews have all been resolved. Thanks!
arduino-ide-extension/src/browser/dialogs/ide-updater/ide-updater-dialog.tsx
Show resolved
Hide resolved
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.
All my comments were addressed! I couldn't find any other issue and the code looks good to me. Thank, Akos!
- Updated `@theia/*` to `1.37.0`. - Fixed all `yarn audit` security vulnerabilities. - Updated to `[email protected]`: - `contextIsolation` is `true`, - `nodeIntegration` is `false`, and the - `webpack` target is moved from `electron-renderer` to `web`. - Updated to `[email protected]`. - Updated the `eslint` plugins. - Added the new `Light High Contrast` theme to the IDE2. - High contrast themes use Theia APIs for style adjustments. - Support for ESM modules: `"moduleResolution": "node16"`. - Node.js >= 16.14 is required. - VISX langage packs were bumped to `1.70.0`. - Removed undesired editor context menu items. (Closes arduino#1394) Signed-off-by: Akos Kitta <[email protected]>
Signed-off-by: Akos Kitta <[email protected]>
Signed-off-by: Akos Kitta <[email protected]>
7534d8a
to
ed007de
Compare
|
Great reviews! Thanks a lot for your time and patience. 🙌 |
Motivation
yarn audit
vulnerabilities,electron
withcontextIsolation
enabled andnodeIntegration
disabled, andChange description
@theia/*
to1.37.0
.yarn audit
security vulnerabilities.[email protected]
:contextIsolation
istrue
,nodeIntegration
isfalse
, and thewebpack
target is moved fromelectron-renderer
toweb
.[email protected]
.eslint
plugins."moduleResolution": "node16"
.1.70.0
.execFileSync
for npm scriptsexeca
for the packager% yarn audit yarn audit v1.22.18 0 vulnerabilities found - Packages audited: 2091 ✨ Done in 1.44s.
Light High Contrast
theme:ide2_hc_light.mp4
Other information
The application is not signed. On macOS, follow these steps.
Closes #321
Closes #1394
Reviewer checklist