chore(deps): Updated to Theia 1.39.0
#2144
Conversation
kittaakos
added
type: enhancement
kittaakos
force-pushed
the
app-packager
branch
3 times, most recently
from
7be3cec
to
2b37c98
Compare
|
✅ Done |
- [security]: removed the app packager - [feat]: bundle the backend app with webpack Signed-off-by: Akos Kitta <[email protected]>
There was a problem hiding this comment.
UPDATE: Fixed by 3529b14
Incorrect layout of editor tab dialog error message highlight
Selecting "New Tab" or "Rename" (if a secondary sketch file tab is selected) from the editor toolbar context menu causes a dialog to open. The user input in the tab name field is validated. If the validation fails, an error message is printed in the dialog.
🐛 The position of the colored background on this text is off-center and doesn't have any padding for the dialog's buttons:
To reproduce
- Select "New Tab" from the editor toolbar context menu.
- In the "Name for new file" field, type
foo bar
The expected error message appears:
The name must start with a letter, number, or underscore, followed by letters, numbers, dashes, dots and underscores. Maximum length is 63 characters.
🐛 but the position of the colored background on this text is off-center and doesn't have any padding for the dialog's buttons.
Expected behavior
The error message highlight is centered on the text and has padding against the buttons.
-OR-
The error message is not highlighted.
Arduino IDE version
090098d (tester build for e8a9ef7)
Operating system
Windows 11
Additional context
The highlighting behavior is something introduced by the PR. There is no such highlight on the error message when using the build from the main branch (f6a4325):
I think it is useful to highlight the error message in some way (though I would prefer it was done in a consistent manner in all the dialogs e.g., Preferences dialog when invalid sketchbook location is selected, Firmware Updater dialog when process fails) to bring it to the attention of the user. However, if fixing the layout is more difficult, I am fine with the alternative fix of removing the highlight altogether as that would not be a regression.
There was a problem hiding this comment.
UPDATE: Fixed by 129436b
Deleting to trash fails
The file of the currently selected editor tab can be deleted by selecting "Delete" from the editor toolbar context menu. The IDE defaults to deleting this via the operating system's "trash" system in order to allow the user to recover the file.
🐛 The attempt to delete to "trash" fails and the user is forced to use the fallback of permanent deletion.
To reproduce
- Select "New Tab" from the editor toolbar context menu.
The "Name for new file" dialog will open. - Type
Fooin the field of the "Name for new file" dialog. - Click the "OK" button.
- Select "Delete" from the editor toolbar context menu.
The "Move File to Trash" dialog will open. - Click the "OK" button in the "Move File to Trash" dialog.
🐛 The attempt to delete the file fails:
Error deleting file
Failed to delete "Foo.ino" using the Trash. Do you want to permanently delete instead?
You can disable the use of Trash in the preferences.
Expected behavior
Deletion via the "trash" system works.
Arduino IDE version
090098d (tester build for e8a9ef7)
Operating system
Windows 11
Additional context
The following is printed to the logs when the deletion fails:
2023-07-30T12:08:32.365Z root ERROR Error deleting with trash: Error: Request 'delete' failed
at Proxy.<anonymous> (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:3032271)
at b.delete (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:3741400)
at x.doDelete (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:3664211)
at async x.delete (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:3663346)
at async d.moveFileToTrash (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:8661047)
at async Promise.all (index 1)
at async d.delete (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:8660793)
at async Promise.all (index 0)
at async d.execute (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:8659279)
at async g.executeCommand (file:///E:/incoming/review/2144/1-090098d/resources/app/lib/frontend/bundle.js:2:2954564)
Caused by: EntryNotFound (FileSystemError): Error: spawn E:\incoming\review\2144\1-090098d\resources\app\lib\backend\windows-trash.exe ENOENT
at u (E:\incoming\review\2144\1-090098d\resources\app\lib\backend\main.js:2:1306074)
at T.toFileSystemProviderError (E:\incoming\review\2144\1-090098d\resources\app\lib\backend\main.js:2:1329976)
at T.delete (E:\incoming\review\2144\1-090098d\resources\app\lib\backend\main.js:2:1324688)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async g.onRequest (E:\incoming\review\2144\1-090098d\resources\app\lib\backend\main.js:2:1064552)
at async c.handleRequest (E:\incoming\review\2144\1-090098d\resources\app\lib\backend\main.js:2:1055445)
I used a secondary sketch file in the demo. If the primary sketch file is selected, the deletion fails silently.
Deletion to trash works as expected when using the IDE build from the main branch (f6a4325).
reverted DOM structure changes in single text input dialogs. temporary workaround before aligning the validation style in all dialogs Ref: eclipse-theia/theia#12585 Signed-off-by: Akos Kitta <[email protected]>
Thanks for spotting it! I agree, and I will take care of it in another PR. For now, I reverted (3529b14) the changes IDE2 pulled in from upstream: 
|
Thanks for the excellent review. See eclipse-theia/theia#12780. |
|
IDE2 will lose the default value generated for the |
revert packager script. ``` gyp info using [email protected] | darwin | x64 ``` ``` error [email protected]: The engine "node" is incompatible with this module. Expected version ">=16.14.0 <17". Got "18.16.1" ```
This should work now with the latest build from this PR. Resolved via e506b6f. I have verified it on macOS; the path was slightly different: |
There was a problem hiding this comment.
UPDATE: Fixed by 75480bd
"Date" field of "about" dialog uses fallback
The build date is shown in the "Date" field of the "about" dialog. The dialog code is written so that it falls back on using the value of "dev build" in this field if the build date data is not available.
🐛 This fallback value is being used in the field in the standard beta tester build generated by the repository's continuous development system.
To reproduce
- Download the beta tester build for the PR.
- Start the beta tester build.
- Select Help > About Arduino IDE from Arduino IDE menus.
The "Arduino IDE" dialog will open.
🐛 The "Date" field is set to "dev build":
Expected behavior
The "Date" field is always set to the build timestamp when the build is generated by standard processes.
Arduino IDE version
3fc51f4 (tester build for 28bc470)
Operating system
Windows 11
Additional context
I haven't found the data from this field to be essential, so if there is some difficulty in populating it specifically for beta tester builds then I am OK with it being set to "dev build" for these builds. My concern is that this is an indicator that the system for populating the field is completely broken and it will be set to this fallback value even for the release builds, which would make the field useless and also possibly cause the users confusion.
merge in the frontend config and any other metadata from the final app's package.json, as those are not available at theia build time. Signed-off-by: Akos Kitta <[email protected]>
|
I do not know what's been wrong with GitHub recently, but the commits do not appear on the PR. Here is my commit (75480bd) on the top of the branch, but it's missing from the PR, and the build has not run either. 
Anyway, if the build starts, please check the fixes. I verified it with a locally bundled app on macOS, and it worked. Thanks! |
Ref: eclipse-theia/theia#12793 Signed-off-by: Akos Kitta <[email protected]>
There was a problem hiding this comment.
UPDATE: Fixed by 920a4fc
When I perform the instructions under "Build from source" in docs/development.md using a fresh clone of the repository checked out to this PR branch (f7cb8aa), step (4) fails:
Failed to start the electron application.
Error: Cannot find module './cli-protocol/cc/arduino/cli/settings/v1/settings_pb'
Require stack:
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/node/config-service-impl.js
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/node/sketches-service-impl.js
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/electron-main/theia/electron-main-application.js
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/electron-main/electron-arduino.js
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/electron-main/arduino-electron-main-module.js
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/electron-app/src-gen/backend/electron-main.js
- /mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/node_modules/electron/dist/resources/default_app.asar/main.js
-
at Module._resolveFilename (node:internal/modules/cjs/loader:1002:15)
at n._resolveFilename (node:electron/js2c/browser_init:2:109734)
at Module._load (node:internal/modules/cjs/loader:848:27)
at f._load (node:electron/js2c/asar_bundle:2:13330)
at Module.require (node:internal/modules/cjs/loader:1068:19)
at require (node:internal/modules/cjs/helpers:103:18)
at Object.<anonymous> (/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/node/config-service-impl.js:50:23)
at Module._compile (node:internal/modules/cjs/loader:1174:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1229:10)
at Module.load (node:internal/modules/cjs/loader:1044:32) {
code: 'MODULE_NOT_FOUND',
requireStack: [
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/node/config-service-impl.js',
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/node/sketches-service-impl.js',
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/electron-main/theia/electron-main-application.js',
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/electron-main/electron-arduino.js',
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/arduino-ide-extension/lib/electron-main/arduino-electron-main-module.js',
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/electron-app/src-gen/backend/electron-main.js',
'/mnt/1c0bd221-e077-452c-b688-160dc700c122/git/arduino-ide_fresh-clone/node_modules/electron/dist/resources/default_app.asar/main.js',
undefined
]
}
There is no arduino-ide-extension/lib/node/cli-protocol folder.
I don't have this problem when I run the same procedure in the repository where I had previously ran Arduino IDE from source with the repository checked out to the main branch.
arduino-ide-extension/src/node/i18n/arduino-localization-contribution.ts
Outdated
Show resolved
Hide resolved
…ibution.ts Co-authored-by: per1234 <[email protected]>
Co-authored-by: per1234 <[email protected]>
Co-authored-by: per1234 <[email protected]>
Co-authored-by: per1234 <[email protected]>
Co-authored-by: per1234 <[email protected]>
- fixed: added missing build step - fixed: reordered sections: Windows tool must come before build part - added: prerequisites link to Theia and link to recommended tool: Code - added: how to clone repo and change dir to repo root - added: section how to start code + link to further docs Signed-off-by: Akos Kitta <[email protected]>
|
Thank you for the review! I have addressed the outstanding change requests and, most importantly, fixed the dev docs by adding the missing step. It should work now. |
Co-authored-by: per1234 <[email protected]>
Co-authored-by: per1234 <[email protected]>
Motivation
1.39.0.webpack:requirecalls.% yarn audit yarn audit v1.22.18 0 vulnerabilities found - Packages audited: 1984 ✨ Done in 2.14s.Other noteworthy changes:
yarncaching in the CI.npmscripts. From now on, executingyarnwon't build the code, but there is a series of scripts to execute to build, test, and bundle the application.arduino-ide-extension/buildtoarduino-ide-extension/src/node/resources.arduino-ide-extension/package.jsonunder thearduinoproperty. From now on, the executable keys match the executable filename.@postConstructmethods to synchronous. The latest InversifyJS version requires it (inversify/InversifyJS#1482).lerna.Change description
Other information
As the PR author, I expect no functional changes in IDE2 after merging this PR.
TODO for @kittaakos:
open sketch in a new windowdoes not reveal the sketch folder node in the tree in the new window.check whether this PR is the breaking change.The defect is unrelated to this PR. SeeOpen Sketch in New Windowdoes not reveal the sketch folder tree node #2145.Reviewer checklist