Filesystem dock folder handling #11428

ghost · 2017-09-20T00:56:33Z

Issues #11336 and #9854 raise a point about not being able to do some common tasks from within the file system dock.

Folders now support the following on right click:

Move
Rename (The "Move or Rename" option was split, as a save as dialog doesn't make sense for folders)
Delete
Copy path (For consistency with files right click)
Open in explorer (For consistency with files right click)
New folder

Fixes the following issues:

Dragging a folder in the folder tree could cause broken dependencies (possibly cause of FileSystem drag and drop can easily lead to missing dependencies issues #5684)
If a move or rename fails, dependencies could be updated anyway
Files which were going to be deleted could also be listed as having broken dependencies in the dependency removal dialog.

This PR has also had to rework the rename & move code to fix bugs with/better support folders, as a result I have also spent some time tidying up and removing duplicated code in these methods. I also have reworked the DependencyRemoveDialog to properly support removing folders.

ghost · 2017-09-22T01:18:22Z

Reworked the DependencyRemovedDialog to support deleting folders. Files that were in a folder are listed under a tree item underneath the folder. There are now some icons for the items in the tree too which makes it look a little nicer:

This commit also fixes a usability bug encountered during testing: Files which are being deleted may be listed as having broken dependencies if you select the file and one or more dependencies with multi-select before deleting.

Finally, because DirAccess only lets you delete empty folders (at least on windows), folder deletion happens in two stages. Files from a list generated when the dialog was opened are removed, then the remaining empty directories are cleaned up. If the user adds a file to a directory while the dialog is open the folder will not be emptied by the first step, so the deleting the directory will fail. I figured this was preferable to doing a second enumeration, as it is possible the new file has become a dependency.

Xgor · 2017-09-23T10:56:57Z

Was thinking of adding it in myself but I'm glad somebody else is. This better be included in 3.0 as this is probably my biggest problem with 2.1 right now. Took and checked it out and it seems to work great. The only problem I have so far is that when you drag a folder it doesn't show what folder is being dragged. I might try adding it myself but I haven't contributed yet so the Godot code is unfamiliar to me right now.

akien-mga · 2017-09-25T14:39:23Z

editor/filesystem_dock.cpp

-	if (all_can_reimport && types.size() == 1) { //all can reimport and are of the same type
-
-		/*
+	/*


Don't keep dead code.

ghost · 2017-09-25T20:15:01Z

I have decided that since I have reworked a the rename/move logic I think i should take a second look at it to fix the following two issues:

(Regression) moving a folder into a child folder will remap dependencies but not move the folder. There used to be a check for this and there isn't anymore.
More generally - if a rename or move fails the dependency remap happens anyway (this also happens in the old code). The solution to this may be to only add the remaps after the operation has been successful, then apply them after the rescan.

Whilst i'm there i also think i can do a better job at deduplicating code between move and rename.

groud · 2017-09-25T22:01:15Z

Some feedback from PR monday:

We are very happy for thoses changes, we hope you'll manage to fix the remaining issues :)
It will probably need a rebase to use the new move_to_thrash function for deletion.

akien-mga · 2017-09-25T22:02:03Z

Sounds like 1b4f7c7 could be squashed into 728688e, and f42764b would go into 1da4149 I guess? See http://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#mastering-the-pr-workflow-the-rebase for docs on interactive rebase.

ghost · 2017-09-25T22:53:47Z

Thanks for the feedback and help so far. I am going to take a look at rebasing and shuffling around a few of these commits down tonight/tomorrow. I hope to have something that will be as follows:

Enabling the right click menu on folders
Move/Rename
Delete
Add new folder

I can squash down further after that more easily if required.
When the move_to_trash function becomes I can rebase once more if necessary.

ghost · 2017-10-01T01:14:01Z

Before I rebase it would be helpful if I could get some thoughts on the latest commit. I am not sure if it is ok to use the EditorFileSystem the way I am here.

The latest commit looks at the EditorFileSystem after the moves/renames are applied but before a call to rescan is made. This is making the assumption that the EditorFileSystem will still have the old file paths & dependencies. Doing this simplifies the code quite a bit as I don't have to keep track of this myself (which is more compicated - especially because I don't know what files will succeed in moving/renaming until actually doing it). The code I have works, but not sure if this is a reasonable requirement to put on the EditorFileSystem.

reduz · 2017-10-01T01:19:43Z

yeah, i guess calling to rescan later is fine

…

On Sat, Sep 30, 2017 at 10:14 PM, MillionOstrich ***@***.***> wrote: Before I rebase it would be helpful if I could get some thoughts on the latest commit. I am not sure if it is ok to use the EditorFileSystem the way I am here. The latest commit looks at the EditorFileSystem after the moves/renames are applied but before a call to rescan is made. This is making the assumption that the EditorFileSystem will still have the old file paths & dependencies. Doing this simplifies the code quite a bit as I don't have to keep track of this myself (which is more compicated - especially because I don't know what files will succeed in moving/renaming until actually doing it). The code I have works, but not sure if this is a reasonable requirement to put on the EditorFileSystem. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#11428 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF-Z22jR93m9pGQVn6d7NjBsRzxWu7Ozks5snudigaJpZM4PdM_m> .

ghost · 2017-10-01T23:13:10Z

Rebased into the 4 commits & added a more detailed comment about the use of EditorFileSystem before rescan while I was there.

ghost · 2017-10-02T21:32:44Z

Rebased and updated this PR following merge of #11575.

Note that as a result of the rebase and changes to how the removal code works there is a bug introduced in the first commit which is fixed in the third. The changes to the right click in the first commit will let you delete ".." which is checked for in the third commit. The justification for this was that a multiselection including ".." might just want skip this selected entry and try to delete the other stuff, but on reflection I think just not letting you pick some options if ".." is selected is better. Does anybody have a preference either way?

groud · 2017-10-02T22:12:35Z

To be honest is see no reason why '..' should be selected. Maybe that should be made impossible before ?

Disallow selecting ".." in the file-system dock. Show In Explorer just uses this->path rather than trying to work it out from the item. Add support for copy to path to folders . Removed old commented out code.

Move/rename don't depend on the path variable anymore. Fixed dependencies not updating correctly when dragging folders in the folder tree. Dependencies will only update for files which sucessfully moved. Reduced code duplication between move & rename. Added rename & move options to folders tree.

DependencyRemoveDialog now takes two lists (files and folders) to delete. Sort the folders above files in DependencyRemoveDialog & use some more icons. Stop files which will be deleted from also being listed as having broken dependencies. Add right-click option for removing folder to filesystem folder tree.

ghost · 2017-10-09T14:18:47Z

I have now made it impossible to have ".." selected (you can still right click it and double click it) and rebased this change into the first commit, fixing the issue raised in my previous comment.

The point Xgor raised I think I will address in a new PR at a later date - along with a different issue I encountered with drag (dragging folders onto the favourites icon doesn't work - it probably should favourite the folder though).

ghost added enhancement topic:editor labels Sep 20, 2017

ghost added this to the 3.0 milestone Sep 20, 2017

ghost force-pushed the filesystem-dock-folders branch from fd93cfc to 728688e Compare September 20, 2017 20:36

akien-mga reviewed Sep 25, 2017

View reviewed changes

ghost force-pushed the filesystem-dock-folders branch from 337c0f9 to 9f6f0c7 Compare October 1, 2017 23:10

ghost force-pushed the filesystem-dock-folders branch from 9f6f0c7 to 32255e2 Compare October 2, 2017 21:15

MillionOstrich added 4 commits October 9, 2017 14:59

Can now right click folders in filesystem dock.

e1fb38e

Disallow selecting ".." in the file-system dock. Show In Explorer just uses this->path rather than trying to work it out from the item. Add support for copy to path to folders . Removed old commented out code.

Add right click option to create folders.

0939c0a

ghost force-pushed the filesystem-dock-folders branch from 2766e71 to 0939c0a Compare October 9, 2017 14:16

ghost changed the title ~~[WIP] Filesystem dock folder handling~~ Filesystem dock folder handling Oct 9, 2017

akien-mga merged commit e498f01 into godotengine:master Oct 15, 2017

akien-mga mentioned this pull request Oct 15, 2017

Moving resources preserves import settings. #11070

Closed

ghost mentioned this pull request Oct 24, 2017

Improved file/folder drag preview for filesystem dock #12390

Merged

ghost deleted the filesystem-dock-folders branch October 24, 2017 22:43

ghost mentioned this pull request Oct 25, 2017

Filesystem drag & drop moving fixes #12402

Merged

HummusSamurai mentioned this pull request Nov 24, 2017

FileSystem drag and drop can easily lead to missing dependencies issues #5684

Closed

guilhermefelipecgs mentioned this pull request Jan 31, 2018

Click on ".." folder in Godot editor FileSystem dock does not indicate if the folder is clicked. #16253

Closed

mhilbrunner mentioned this pull request Apr 10, 2018

Mouse right click on filesystem's panel #11336

Closed

timothyqiu mentioned this pull request Oct 30, 2024

Remove unused FILE_INFO from FileSystem dock #98662

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filesystem dock folder handling #11428

Filesystem dock folder handling #11428

ghost commented Sep 20, 2017 •

edited by ghost

Loading

ghost commented Sep 22, 2017

Xgor commented Sep 23, 2017

akien-mga Sep 25, 2017

ghost commented Sep 25, 2017

groud commented Sep 25, 2017 •

edited

Loading

akien-mga commented Sep 25, 2017

ghost commented Sep 25, 2017

ghost commented Oct 1, 2017

reduz commented Oct 1, 2017 via email

ghost commented Oct 1, 2017

ghost commented Oct 2, 2017 •

edited by ghost

Loading

groud commented Oct 2, 2017

ghost commented Oct 9, 2017 •

edited by ghost

Loading

Filesystem dock folder handling #11428

Filesystem dock folder handling #11428

Conversation

ghost commented Sep 20, 2017 • edited by ghost Loading

ghost commented Sep 22, 2017

Xgor commented Sep 23, 2017

akien-mga Sep 25, 2017

Choose a reason for hiding this comment

ghost commented Sep 25, 2017

groud commented Sep 25, 2017 • edited Loading

akien-mga commented Sep 25, 2017

ghost commented Sep 25, 2017

ghost commented Oct 1, 2017

reduz commented Oct 1, 2017 via email

ghost commented Oct 1, 2017

ghost commented Oct 2, 2017 • edited by ghost Loading

groud commented Oct 2, 2017

ghost commented Oct 9, 2017 • edited by ghost Loading

ghost commented Sep 20, 2017 •

edited by ghost

Loading

groud commented Sep 25, 2017 •

edited

Loading

ghost commented Oct 2, 2017 •

edited by ghost

Loading

ghost commented Oct 9, 2017 •

edited by ghost

Loading