joshwcomeau / guppy

🐠A friendly application manager and task runner for React.js
ISC License
3.27k stars 154 forks source link

Show dialog if project delete fails #279

Closed AWolf81 closed 5 years ago

AWolf81 commented 5 years ago

If a user tries to delete a project from disk and if deleting fails there is no message about the failure. It looks like everything is working as expected but it wasn't.

E.g. if a project folder is open inside VS code the deletion of a project will fail with-out any message. Also the console.error from delete-project.saga won't be called at line 94. It seems like it will always return success from moveItemToTrash. I've seen this on Windows.

Describe the solution you'd like Show a dialog that project folder couldn't be deleted. Node_modules folder could be deleted but not the project folder. Stop removing the project from Guppy. At the moment, the project still exists on disk but will be removed from Guppy.

Additional context Delay this task until #277 is finished as there is some work in progress at project delete.

rmorabia commented 5 years ago

I'm going to move myself over here & say I'd be happy to take this on!

To clarify: The solution is to:

I don't understand what this part of the issue means: "Stop removing the project from Guppy. At the moment, the project still exists on disk but will be removed from Guppy."

Stop the deletion process, and then what should the function do? Nothing? Or something in particular to keep it on the disk but removed from Guppy?

Thank you!

melanieseltzer commented 5 years ago

"Stop removing the project from Guppy. At the moment, the project still exists on disk but will be removed from Guppy."

Hi @rmorabia! I think what that means is that currently, deleting from disk fails silently as described above, but then line 112 and beyond still runs which proceeds to remove the project from the Guppy app, even if deleting from disk fails. This shouldn't happen - if moveItemToTrash fails, the project should remain in the Guppy app and the on screen errors thrown so the user can try again.

rmorabia commented 5 years ago

Thanks for the clarification @melanieseltzer!

I plan to work on this this weekend, but if I don't finish -- I'm also heading to a conference next week, so if I disappear, I'd still like to finish this after I come back. Thanks!

melanieseltzer commented 5 years ago

Sounds good 😄

melanieseltzer commented 5 years ago

While working on #294 we ran into this again, and I just realized that on a Mac (at least for me) I'm unable to make a delete fail. I don't run into the ENOTEMPTY issue since it's Windows specific, and delete still works for me even if the project files/folder is open in VS Code (I'm not sure why that is).

@AWolf81 Do you know if there a reliable way to make a delete fail in case @rmorabia is on Mac and cannot reproduce a failure?

AWolf81 commented 5 years ago

@melanieseltzer not sure, maybe opening a terminal and cd to the project folder. Then start devServer with yarn start. This could block move to trash. To block node_modules deletion: On Windows I can open a terminal and then cd into the node_modules folder. That will block the deletion - maybe that could work for Mac as well.

rmorabia commented 5 years ago

I'm sure I can add some permission stuff through Bash that block the folders from being deleted.

rmorabia commented 5 years ago

Update: Sorry about this, but I think I'm going to have to abandon this issue. It's just not the right time. Looking forward to contributing again when things slow down a bit. Thanks!

melanieseltzer commented 5 years ago

No worries, @rmorabia!