Sketch deleted when location inside sketch folder selected as rename target

[per1234]

Describe the problem

The Arduino IDE editor toolbar contains a "context" menu accessed via the ●●● icon. This menu includes a "Rename" option. The behavior of the "Rename" option depends on which type of sketch file tab is selected in the editor:

• If it is a secondary file, the option renames the file alone
• If it is the primary .ino file (the leftmost tab), the file and sketch folder are renamed

🐛 If the user selects a location inside the current sketch folder when renaming the primary tab, the sketch may be deleted.

To reproduce

1. Select File > New Sketch from the Arduino IDE menus.
2. Select File > Save As... from the Arduino IDE menus.
3. Save the sketch with the name Foo
4. Click on the "Foo.ino" tab in the Arduino IDE editor.
   ⓘ This step is required due to No delete/rename menu actions from the editor toolbar if editor has no focus after app start #1847
5. Click the ●●● icon on the right side of the editor toolbar.
   The editor context menu will open.
6. Select "Rename" from the menu.
   The "Save sketch folder as..." dialog will open.
7. Select the Foo folder from the navigation panel of the dialog.
8. Save the sketch as Bar

🐛 Intermittently, the above procedure will cause the the Bar sketch to be deleted along with the Foo parent folder.

The other times it will not be deleted, but the logs show that the IDE still attempted the deletion:

2023-02-11T22:29:01.644Z sketches-service ERROR Failed to delete sketch at c:\Users\per\Documents\Arduino\Foo.
2023-02-11T22:29:01.645Z root ERROR Request deleteSketch failed with error: EBUSY: resource busy or locked, rmdir 'c:\Users\per\Documents\Arduino\Foo\Bar'
2023-02-11T22:29:01.694Z root ERROR Error occurred when executing the startup task 'arduino-delete-sketch' with args: '["file:///c%3A/Users/per/Documents/Arduino/Foo"].

The intermittent outcome seems to be dependent on the timing of whether the deletion of the Foo folder is completed before the IDE opens the Foo/Bar/Bar.ino file. If the former, the sketch is deleted. If the latter, the deletion process fails due to the IDE creating a lock on it by having the sketch open.

Expected behavior

There is no valid use case for this procedure. The user will only attempt the above procedure based on a misunderstanding of the nature on an Arduino sketch and how the IDE's "Rename" feature works when done on the primary sketch file. These users expect the outcome to be that the primary sketch file alone is renamed, resulting in this structure:

Foo/
└── Bar.ino

However, the IDE's "Rename" feature intentionally does not work this way due to the requirement that the primary .ino file match the sketch folder name.

So the ideal behavior would be for Arduino IDE to:

• Prevent the sketch from being be renamed to a location inside its previous sketch folder.
• Clearly communicate to the user that this is not allowed.

---

If the above is not possible, the second best behavior would be to end with a structure like this:

Foo/
└── Bar/
    └── Bar.ino

This will not be the user's expected outcome, but the sketch is not deleted and remains valid.

Arduino IDE version

79b6b7e

Operating system

Windows

Operating system version

11

Additional context

I am also able to reproduce the fault using the tester build from #1833 (b9e9ae9). I found the fault was no longer intermittent, but sometimes has a different characteristic in that the sketch is deleted after existing the IDE. It seems the deletion is postponed when the IDE is not able to complete the process due to a lock:

>>> Finishing scheduled sketch deletions before app quit...
>>> Running sketch deletion c:\Users\per\Documents\Arduino\Foo before app quit...
<<< Deleted sketch c:\Users\per\Documents\Arduino\Foo
<<< Successfully finishing scheduled sketch deletions.

---

Originally reported at https://forum.arduino.cc/t/arduino-ide-deleted-all-project-files-after-changing-from-ino-to-cpp/1088672

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

[DerangedMind]

@per1234 thanks for creating this issue on my behalf! I read through your analysis and I'm concerned that the proposed solution is a bandage on the underlying problem.

The way I'm seeing the problem, if the main .ino file is not found, it will trigger a sketch project initialization which results in the entire file path being reinitialized.

After looking through some of the source code, I found this in the sketch init service:

arduino-ide/arduino-ide-extension/src/node/sketches-service-impl.ts

Line 89 in 658f117

async getSketches({ uri }: { uri?: string }): Promise<SketchContainer> {

I believe a solution that would resolve the punishment for this action would be easier to implement. In addition, a function named "getSketches" implies no side-effects, but it's clearly doing some heavy lifting here. Ideally, this should simply do a check and pass the responsibility of resolution to the caller.

I can take a look at this later if this solution sounds promising. I've got many years of experience working with Theia, so I'm happy to make a contribution, just let me know. Thanks again!

[kittaakos]

So the ideal behavior would be for Arduino IDE to:

• Prevent the sketch from being be renamed to a location inside its previous sketch folder.
• Clearly communicate to the user that this is not allowed.

IDE2 can do the same as IDE 1.x:

[kittaakos]

8. Save the sketch as Bar

🐛 Intermittently, the above procedure will cause the the Bar sketch to be deleted

IDE2 must disallow nested sketch destination location if the source sketch should be deleted, but the real problem is here:

arduino-ide/arduino-ide-extension/src/browser/theia/workspace/workspace-commands.ts

Line 150 in 79b6b7e

wipeOriginal: true,

When renaming an ordinary, non-temp sketch, IDE2 must not wipe the source sketch as the operation is save-as.

[per1234]

I've got many years of experience working with Theia, so I'm happy to make a contribution

Very cool that you are experienced with Theia @DerangedMind! Contributions are always welcome.

In this specific case I would suggest you coordinate with @kittaakos before starting any serious work on a fix just so we avoid any chance of wasting your time due to duplicate efforts. I see @kittaakos already added a fix at bec6e92

[per1234]

IDE2 can do the same as IDE 1.x:

Even though I appreciate the sentiment behind the dialog title, I don't think it is appropriate in this context. The user who triggers this dialog is already confused about the nature of Arduino sketches and is unlikely to be in the mindset to appreciate an obscure "Borges" reference. They may try to interpret it from a technical (vs. humorous) perspective and become even more confused.

I would also drop the "this would go on forever" part. This is referencing a potential (it doesn't exist in Arduino IDE 2.x) defect in the application which would cause it to create an infinitely recursive subfolder structure until something breaks in the file/operating system. So this implies that the restriction is only a workaround for a defect instead of an intentional design.

But as far as the idea of using a dialog to communicate the situation to the user, I'm fine with that.

When renaming an ordinary, non-temp sketch, IDE2 must not wipe the source sketch as the operation is save-as.

I'm not sure about this. A rename operation is equivalent to a move, where the source is removed. That is the current behavior of the IDE's sketch rename feature. You are proposing changing it to a copy operation.

[kittaakos]

Even though I appreciate the sentiment behind the dialog title, I don't think it is appropriate in this context.

We're on the same page 👍

I would also drop the "this would go on forever" part.

I am open to any suggestions.

I'm not sure about this.

It's clearly a bug with IDE2. Check with IDE 1.x:

save_as_is_not_move.mp4

As a user of any software application, for me, save as does not mean moving and losing the original content but if you think IDE2 should behave differently, I can keep the current logic and let IDE2 wipe the original sketch content after save as.

[per1234]

Check with IDE 1.x:

You are not performing a "rename" in that screencast. Try a rename in Arduino IDE 1.x and see what happens:

As a user of any software application, for me, save as does not mean moving and losing the original content

I agree, but this is not about a "Save As" operation, even if Arduino IDE 2.x is reusing the "Save As" infrastructure. The user is performing a "rename" operation, so they will expect the move behavior that is standard for any rename.

[kittaakos]

You're the best. Thank you for the correction! It's not a bug.

[DerangedMind]

@per1234 thanks for creating this issue on my behalf! I read through your analysis and I'm concerned that the proposed solution is a bandage on the underlying problem.

The way I'm seeing the problem, if the main .ino file is not found, it will trigger a sketch project initialization which results in the entire file path being reinitialized.

After looking through some of the source code, I found this in the sketch init service:

arduino-ide/arduino-ide-extension/src/node/sketches-service-impl.ts

Line 89 in 658f117

async getSketches({ uri }: { uri?: string }): Promise<SketchContainer> {

I believe a solution that would resolve the punishment for this action would be easier to implement. In addition, a function named "getSketches" implies no side-effects, but it's clearly doing some heavy lifting here. Ideally, this should simply do a check and pass the responsibility of resolution to the caller.

I can take a look at this later if this solution sounds promising. I've got many years of experience working with Theia, so I'm happy to make a contribution, just let me know. Thanks again!

[kittaakos]

So the ideal behavior would be for Arduino IDE to:

• Prevent the sketch from being be renamed to a location inside its previous sketch folder.
• Clearly communicate to the user that this is not allowed.

IDE2 can do the same as IDE 1.x:

[kittaakos]

8. Save the sketch as Bar

🐛 Intermittently, the above procedure will cause the the Bar sketch to be deleted

IDE2 must disallow nested sketch destination location if the source sketch should be deleted, but the real problem is here:

arduino-ide/arduino-ide-extension/src/browser/theia/workspace/workspace-commands.ts

Line 150 in 79b6b7e

wipeOriginal: true,

When renaming an ordinary, non-temp sketch, IDE2 must not wipe the source sketch as the operation is save-as.

[per1234]

I've got many years of experience working with Theia, so I'm happy to make a contribution

Very cool that you are experienced with Theia @DerangedMind! Contributions are always welcome.

In this specific case I would suggest you coordinate with @kittaakos before starting any serious work on a fix just so we avoid any chance of wasting your time due to duplicate efforts. I see @kittaakos already added a fix at bec6e92

[per1234]

IDE2 can do the same as IDE 1.x:

Even though I appreciate the sentiment behind the dialog title, I don't think it is appropriate in this context. The user who triggers this dialog is already confused about the nature of Arduino sketches and is unlikely to be in the mindset to appreciate an obscure "Borges" reference. They may try to interpret it from a technical (vs. humorous) perspective and become even more confused.

I would also drop the "this would go on forever" part. This is referencing a potential (it doesn't exist in Arduino IDE 2.x) defect in the application which would cause it to create an infinitely recursive subfolder structure until something breaks in the file/operating system. So this implies that the restriction is only a workaround for a defect instead of an intentional design.

But as far as the idea of using a dialog to communicate the situation to the user, I'm fine with that.

When renaming an ordinary, non-temp sketch, IDE2 must not wipe the source sketch as the operation is save-as.

I'm not sure about this. A rename operation is equivalent to a move, where the source is removed. That is the current behavior of the IDE's sketch rename feature. You are proposing changing it to a copy operation.

[kittaakos]

Even though I appreciate the sentiment behind the dialog title, I don't think it is appropriate in this context.

We're on the same page 👍

I would also drop the "this would go on forever" part.

I am open to any suggestions.

I'm not sure about this.

It's clearly a bug with IDE2. Check with IDE 1.x:

save_as_is_not_move.mp4

As a user of any software application, for me, save as does not mean moving and losing the original content but if you think IDE2 should behave differently, I can keep the current logic and let IDE2 wipe the original sketch content after save as.

[per1234]

Check with IDE 1.x:

You are not performing a "rename" in that screencast. Try a rename in Arduino IDE 1.x and see what happens:

As a user of any software application, for me, save as does not mean moving and losing the original content

I agree, but this is not about a "Save As" operation, even if Arduino IDE 2.x is reusing the "Save As" infrastructure. The user is performing a "rename" operation, so they will expect the move behavior that is standard for any rename.

[kittaakos]

You're the best. Thank you for the correction! It's not a bug.