Add `editor.delete_directory()` #8473

astrochili · 2024-01-30T10:06:53Z

Technical changes

This changeset adds a new function to editor scripts that allows the user to delete an existing directory. The function expects a resource path, i.e. a /-prefixed path from the root of the project, for example:

editor.delete_directory("/assets/generated")

astrochili · 2024-01-30T10:13:18Z

I created it by looking at the PR #8434

Haven't figured out how to build the editor locally yet, but the code seems simple. Please correct me if something is wrong 🙈.

astrochili · 2024-01-30T10:39:57Z

defold/doc#383

britzl · 2024-01-30T11:40:10Z

Haven't figured out how to build the editor locally yet, but the code seems simple. Please correct me if something is wrong 🙈.

I think it's good if you try to set up the editor so that you can build it. Submitting PRs "in the blind" so to speak is not a good practice :-) What kind of problems did you have when trying to set everything up?

AGulev · 2024-01-30T11:54:34Z

I've just tested it and it doesn't work:

ERROR:EXT: hook "on_target_terminated" in /hooks.editor_script failed:
ERROR:EXT: /hooks.editor_script:5 class sun.nio.fs.UnixPath cannot be cast to class java.io.File (sun.nio.fs.UnixPath and java.io.File are in module java.base of loader 'bootstrap')

astrochili · 2024-01-30T11:58:48Z

Sorry for your time, yes, that was brave 🫣. Gone to build the editor...

AGulev · 2024-01-30T12:11:01Z

Sorry for your time, yes, that was brave 🫣. Gone to build the editor...

We have #source-code channel in Discord. Feel free to ask your questions there. We will help you to build the engine (the first step) and the editor.

matgis · 2024-01-30T12:41:03Z

No need to build the engine if you're making a change that only affects the editor in isolation. The lein init archived-stable command will download the latest stable engine and Bob binaries for you so you can skip setting up the whole engine build environment. Just make sure to prefix your branch name with DEFEDIT- and target the editor-dev branch with your PR. If your PR is accepted, it will be merged into the next editor update, so no need to wait for the next stable Defold release.

More info about the lein init archived-stable command here:
https://github.com/defold/defold/blob/dev/editor/README_BUILD.md#build-using-existing-engine-and-tools

AGulev · 2024-01-30T12:42:47Z

@matgis oh, I didn't know that. Nice!

JCash · 2024-01-30T16:57:04Z

Thanks for the PR!
I don't mean to pile on here, but perhaps @matgis could direct you in how to add a small test as well?

matgis · 2024-01-30T17:39:14Z

@JCash I would feel more justified in asking for a test if our code they based this on had tests. 😅 Personally I think we can add tests later.

astrochili · 2024-01-30T21:57:12Z

More info about the lein init archived-stable command here:
https://github.com/defold/defold/blob/dev/editor/README_BUILD.md#build-using-existing-engine-and-tools

Thanks. I built the editor and fixed the code. Updated the commit, now directory deletion works 👍

But still not sure if the exceptions I added will work, or if any other checks are needed.

I have not been able to reproduce NoSuchFileException and NotDirectoryException. If there is no directory, nothing happens. If instead of a directory it is a file - the file will be deleted.

defold#8472

astrochili · 2024-01-31T09:25:54Z

UPDATE:

Removed NoSuchFileException because it doesn't sense when deleting.
Fixed NotDirectoryException, now it throws an exception to avoid deleting the file. Because creating and deleting files still available with io.write and os.remove as I understand.

Now there are only issues of code reuse in editor-extensions.clj and consistency of function call chains inside fs.clj. But I'm not really good at this, first time writing in clojure.

matgis · 2024-01-31T18:05:57Z

@astrochili I added a commit with some additional protections against deleting anything below .git or .internal, as well as preventing you from deleting the project directory itself.

If it looks good to you, perhaps you could cherry-pick it into your PR?

f18dec0

astrochili · 2024-01-31T20:12:36Z

@matgis Added a cherry-picked commit, looks good?

matgis

Looks good! I'm going to merge it to editor-dev. Once CI completes, you should get it in the next editor update.

vlaaad · 2024-01-31T21:09:32Z

editor/src/clj/editor/editor_extensions.clj

    (if (.startsWith dir-path root-path)
      (try
        (fs/create-path-directories! dir-path)
        (vreset! request-sync true)
        nil
        (catch FileAlreadyExistsException e
-          (throw (LuaError. (str "File already exists: " (.getMessage e)))))
+          (throw (LuaError. (str "Directory already exists: " (.getMessage e)))))


I'm fairly certain the error message should say "file already exists" and not "directory already exists". Directory already existing is not an error and should not throw exception. What should throw exception is if you wanted a directory at a certain path, and it turned out at that path there is a file instead, that's what the error it's about.

Oh really? I was thinking it's just a typo. So how to fix it back, a new PR? 🤔

defold#8473 (comment)

* Reverted wrong error message renaming #8473 (comment)

astrochili changed the title ~~Add editor.delete_directory()~~ Add editor.delete_directory() Jan 30, 2024

This was referenced Jan 30, 2024

Add 'lua' action to the editor scripts to call a lua function in the sequence. #6810

Open

Document editor.delete_directory() defold/doc#383

Merged

britzl linked an issue Jan 30, 2024 that may be closed by this pull request

Add editor.delete_directory function #8472

Closed

britzl requested a review from matgis January 30, 2024 11:23

astrochili force-pushed the DEFEDIT-8472 branch from f9dbd58 to 9fbef4e Compare January 30, 2024 21:51

Add editor.delete_directory()

150a8da

defold#8472

astrochili force-pushed the DEFEDIT-8472 branch from 9fbef4e to 150a8da Compare January 31, 2024 09:17

Prevent editor.delete_directory() from deleting certain directories

e6894e9

matgis approved these changes Jan 31, 2024

View reviewed changes

matgis added editor Issues related to the Defold editor editor scripts Issue related to editor scripts labels Jan 31, 2024

matgis merged commit 1300e83 into defold:editor-dev Jan 31, 2024
1 check passed

vlaaad reviewed Jan 31, 2024

View reviewed changes

astrochili added a commit to astrochili/defold that referenced this pull request Jan 31, 2024

Reverted wrong error message renaming

2e594d6

defold#8473 (comment)

astrochili mentioned this pull request Jan 31, 2024

Revert back to the File already exists error message #8481

Merged

matgis pushed a commit that referenced this pull request Feb 1, 2024

Revert back to the File already exists error message (#8481)

6b34561

* Reverted wrong error message renaming #8473 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `editor.delete_directory()` #8473

Add `editor.delete_directory()` #8473

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 30, 2024

britzl commented Jan 30, 2024

AGulev commented Jan 30, 2024

astrochili commented Jan 30, 2024 •

edited

AGulev commented Jan 30, 2024

matgis commented Jan 30, 2024

AGulev commented Jan 30, 2024

JCash commented Jan 30, 2024

matgis commented Jan 30, 2024

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 31, 2024 •

edited

matgis commented Jan 31, 2024 •

edited

astrochili commented Jan 31, 2024 •

edited

matgis left a comment

vlaaad Jan 31, 2024

astrochili Jan 31, 2024 •

edited

Add editor.delete_directory() #8473

Add editor.delete_directory() #8473

Conversation

astrochili commented Jan 30, 2024 • edited

Technical changes

astrochili commented Jan 30, 2024 • edited

astrochili commented Jan 30, 2024

britzl commented Jan 30, 2024

AGulev commented Jan 30, 2024

astrochili commented Jan 30, 2024 • edited

AGulev commented Jan 30, 2024

matgis commented Jan 30, 2024

AGulev commented Jan 30, 2024

JCash commented Jan 30, 2024

matgis commented Jan 30, 2024

astrochili commented Jan 30, 2024 • edited

astrochili commented Jan 31, 2024 • edited

matgis commented Jan 31, 2024 • edited

astrochili commented Jan 31, 2024 • edited

matgis left a comment

Choose a reason for hiding this comment

vlaaad Jan 31, 2024

Choose a reason for hiding this comment

astrochili Jan 31, 2024 • edited

Choose a reason for hiding this comment

Add `editor.delete_directory()` #8473

Add `editor.delete_directory()` #8473

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 30, 2024 •

edited

astrochili commented Jan 31, 2024 •

edited

matgis commented Jan 31, 2024 •

edited

astrochili commented Jan 31, 2024 •

edited

astrochili Jan 31, 2024 •

edited