-
-
Notifications
You must be signed in to change notification settings - Fork 399
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
#964 Prompt move when opening invalid sketch outside from IDE2 #1563
Conversation
09b2336
to
6562c15
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.
Dialog is not shown when opening sketch with mismatched file/folder names on Windows
To reproduce
Follow the instructions for (9) above.
🐛 The IDE starts up with a "fallback sketch" without showing a dialog to communicate the problem with the sketch to the user.
Expected behavior
The same behavior as when I attempt to open the file via File > Open:
Arduino IDE version
2.0.1-snapshot-c2f6cd6 (tester build for 37c30d5)
Operating system
Windows
Operating system version
10
Additional context
The IDE is working as expected under these conditions on Linux.
The problem occurs with both absolute and relative sketch paths.
The problem occurs with paths to both the sketch folder and sketch file.
I see this in the daemon logs (in this case I opened the sketch file via file association because the daemon logs output hangs the command line terminal):
2022-10-20 01:26:51 daemon INFO 4 CALLED: /cc.arduino.cli.commands.v1.ArduinoCoreService/LoadSketch
2022-10-20 01:26:51 daemon INFO 4 | REQ: {
4 | "instance": {
4 | "id": 1
4 | },
4 | "sketch_path": "c:\\ide 2\\rev\\untitled folder2"
4 | }
2022-10-20 01:26:51 daemon INFO 4 | ERROR: rpc error: code = NotFound desc = Can't open sketch: main file missing from sketch: C:\ide 2\rev\untitled folder2\untitled folder2.ino
2022-10-20 01:26:51 daemon INFO 4 | RESP: null
4 CALL END
5 CALLED: /cc.arduino.cli.commands.v1.ArduinoCoreService/LoadSketch
5 | REQ: {
5 | "instance": {
5 | "id": 1
5 | },
5 | "sketch_path": "c:\\ide 2\\rev"
5 | }
2022-10-20 01:26:51 daemon INFO 5 | ERROR: rpc error: code = NotFound desc = Can't open sketch: main file missing from sketch: C:\ide 2\rev\rev.ino
5 | RESP: null
5 CALL END
6 CALLED: /cc.arduino.cli.commands.v1.ArduinoCoreService/LoadSketch
6 | REQ: {
6 | "instance": {
6 | "id": 1
6 | },
6 | "sketch_path": "c:\\ide 2"
6 | }
2022-10-20 01:26:51 daemon INFO 6 | ERROR: rpc error: code = NotFound desc = Can't open sketch: main file missing from sketch: C:\ide 2\ide 2.ino
6 | RESP: null
6 CALL END
The first request matches what I see on the Linux machine, but the next two where the IDE attempts to load the parent paths as sketches is unique and strange.
Issue checklist
- I searched for previous reports in the issue tracker
- I verified the problem still occurs when using the latest nightly build
- My report contains all necessary details
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 tested all the cases on MacOS, and here are the results:
- ❌
expected: IDE2 opens and reloads with a fallback sketch. No notifications, no dialogs. A temp sketch is opened with the current date.
actual: IDE2 opens the last open sketch (not a temp sektch) - ✅
- ✅
- ✅
- ✅
- ✅
- ✅
- ✅
- ❌ - I think there's just an error in the description of step 9, because in the premise you say to open
fails_to_open.ino
:
Open the untitled folder2/fails_to_open.ino main sketch file from a terminal.
./Arduino\ IDE.app/Contents/MacOS/Arduino\ IDE ./untitled\ folder2/fails_to_open.ino
and then you refer to sketch.ino
:
Expectations are the same as for step 8. Click on OK. IDE2 reloads and moves the sketch to
./untitled folder2/sketch/sketch.ino
. IDE2 is functional.
Probably you wanted to write:
- (corrected) Open the untitled folder2/sketch.ino main sketch file from a terminal.
./Arduino\ IDE.app/Contents/MacOS/Arduino\ IDE ./untitled\ folder2/sketch.ino
Expectations are the same as for step 8. Click on OK. IDE2 reloads and moves the sketch to
./untitled folder2/sketch/sketch.ino
. IDE2 is functional.
Am I right? In that case,
9. (corrected) ✅
10. ✅
Arduino IDE version
12.0.1-snapshot-c2f6cd6 (tester build for 37c30d5)
Operating system
MacOS
Operating system version
13.3.1 (21E258)
Yes, you're right. Thank you! |
Great review 👍
This has been fixed. ✅ Added additional notes about the expected behavior to the PR description: 👇
Correct. The error was in the description. I have updated the expectation. ✅ I have downloaded the test build and went through the steps. All worked on my macOS installation. Please take another look, and let me know how it works. Thank you! 🙏 |
I did another run of tests with a new build, now 1. is working, but 7. fails. Here are the new results
Result: IDE2 opens and reloads with a fallback sketch. No notifications, no dialogs. A temp sketch is opened with the current date. Screen recordingScreen.Recording.2022-10-20.at.15.31.06.movArduino IDE version12.0.1-snapshot-de4e865 (tester build for 91c7cef) Operating systemMacOS Operating system version13.3.1 (21E258) |
I could not reproduce it. It works here with the build. Could you please double-check if you have picked the correct IDE2 version to open the invalid sketch? 964__step_7.mp4 |
The attached video helped a lot. Yes, you opened the default IDE2, a nightly build by double-clicking on the |
oh, you're right! Sorry for that! |
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.
Everything's working as expected for me.
Also code, looks good to me (I've just left two unimportant remarks)
Thank you Akos!
arduino-ide-extension/src/browser/contributions/open-sketch-files.ts
Outdated
Show resolved
Hide resolved
arduino-ide-extension/src/browser/theia/workspace/workspace-service.ts
Outdated
Show resolved
Hide resolved
@per1234, please take another look if you have time. Thank you! |
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 still experience the same problem as before:
Dialog is not shown when opening sketch with mismatched file/folder names on Windows
To reproduce
- Create a sketch which does not contain a
.ino
file matching the folder name:Foo/ └── Bar.ino
- Open the Arduino IDE installation folder in a command line terminal.
- Pass the path to the sketch with the file/folder name mismatch as an argument to the Arduino IDE invocation:
& ".\Arduino IDE" "C:\Users\per\Documents\Arduino\Foo\Bar.ino"
- Wait for the Arduino IDE to finish starting up.
🐛 The IDE starts up with a "fallback sketch" without showing a dialog to communicate the problem with the sketch to the user.
Expected behavior
The same behavior as when I attempt to open the file via File > Open:
Arduino IDE version
2.0.1-snapshot-6324f75 (tester build for fc4a668)
Operating system
Windows
Operating system version
10
Additional context
The IDE is working as expected under these conditions on Linux.
The problem occurs with both absolute and relative sketch paths.
The problem occurs with paths to both the sketch folder and sketch file.
I could reproduce the defect on Windows and fix it. I have verified the case mentioned in #1563 (review), and it worked. Please review. Thank you! |
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 is working perfectly for me now. I verified it:
- fixes Opening invalid sketch via file association/command line fails silently #964
- resolves Dump IDE version to the log files #1484
- adds support for specifying path to sketch folder instead of file
- adds support for relative sketch paths
Thanks Akos!
Motivation
The sketch folder name must match the name of the main sketch file. See the specs. This PR makes IDE2 more resilient by offering a sketch move to the user. This PR proposes the same behavior as IDE 1.x.
Change description
This PR consists of three changes:
NOT_FOUND
error received from the CLI and converts it into an "invalid sketch" error so that the frontend can handle it gracefully,.ino
, previously only directories were supported in IDE2 as a workspace root from theURL#hash
,This PR also fixes #1484 by printing the version of the IDE2 to the console/logs.
Acceptance criteria:
Folder setup:
.
is thecwd
,File
>Quit
equivalent.Open
empty_workspace
:IDE2 opens and reloads with a fallback sketch. No notifications, no dialogs. A temp sketch is opened with the current date.
Open the
other_folder
sketch folder with a relative path:IDE2 successfully opens the
other_folder
sketch.Open the
other_folder.ino
main sketch file with a relative path:IDE2 successfully opens the
other_folder
sketch.Open the
my_workspace.ino
main sketch file with an absolute path:IDE2 successfully opens the
my_workspace
sketch.Open
my_workspace
with a noncanonical path:IDE2 successfully opens the
my_workspace
sketch.Open IDE2 by clicking on
my_workspace.ino
icon on your OS. Or the context menu, select the appropriate IDE2 version to open with if you have multiple ones installed. Openother_folder.ino
the same way. Close IDE2 with theFile
>Quit
equivalent. Start the IDE2 from the terminal without any sketch paths or from the application icon:IDE2 successfully opens and restores both the
my_workspace
and theother_folder
sketches.Open IDE2 by clicking on the
untitled folder2/fails_to_open.ino
icon. (See step 6. on how to open.)A fallback sketch opens. A modal dialog says that the
fails_to_open.ino
must be in a correctly named sketch folder. There is a notification message showing the original error message from the CLI.Click
cancel
. The fallback sketch is there, and IDE2 is fully functional.Open
untitled folder2/fails_to_open.ino
from a terminal.Expectations are the same as for step 8, but click on
OK
, and expect an error dialog; the folder already exists. Click onOK
. The fallback sketch is there, and IDE2 is functional.Open the
untitled folder2/sketch.ino
main sketch file from a terminal.Expectations are the same as for step 8. Click on
OK
. IDE2 reloads and moves the sketch to./untitled folder2/sketch/sketch.ino
. IDE2 is functional.Rename
./untitled\ folder2/sketch/sketch.ino
to./untitled\ folder2/sketch_old.ino
Start IDE2 either from the application icon or without sketch folder params from a terminal:
IDE2 prompts moving
sketch_old.ino
insidesketch_old/sketch_old.ino
.Handling multiple invalid sketch files:
When opening the IDE2 by pointing to the sketch folder which contains multiple invalid sketches, IDE2 picks the first invalid one.
readdir(3)
does not guarantee an ordering, so it is done bycomparing
the filenames.IDE2 will offer to move
sketch_new.ino
and notsketch_old.ino
.This is open for discussion.
IDE 1.x shows something else besides the dialog:
IDE2 shows the error from the CLI:
When the IDE2 is started from a terminal, the
path
input can be a folder or a file:path
will be used,path
will be used,path
will be used, but IDE2 will reload and will provide a fallback sketch,path
argument and restores the previous sketches, if any. Otherwise, a fallback sketch will open..ino
file, the parent folder will be used as thepath
to open the sketch,.ino
file, IDE2 ignores thepath
argument and restores the previous sketches, if any. Otherwise, a fallback sketch will open.Other information
Closes #964
Closes #1484
Reviewer checklist