diff --git a/src/app/file_system.cpp b/src/app/file_system.cpp index 5af3f5d7a..8aeb1fe98 100644 --- a/src/app/file_system.cpp +++ b/src/app/file_system.cpp @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2019 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -46,15 +47,31 @@ #define MAX_PATH 4096 #endif -#define NOTINITIALIZED "{__not_initialized_path__}" +#define NOTINITIALIZED "{*empty*}" -#define FS_TRACE(...) +#define FS_TRACE(...) // TRACE namespace app { +namespace { + +class FileItem; +typedef std::map FileItemMap; + +// the root of the file-system +FileItem* rootitem = nullptr; +FileItemMap* fileitems_map = nullptr; +unsigned int current_file_system_version = 0; + +#ifdef _WIN32 + IMalloc* shl_imalloc = nullptr; + IShellFolder* shl_idesktop = nullptr; +#endif + // a position in the file-system class FileItem : public IFileItem { public: + // TODO make all these fields private std::string m_keyname; std::string m_filename; std::string m_displayname; @@ -62,7 +79,7 @@ public: FileItemList m_children; unsigned int m_version; bool m_removed; - bool m_is_folder; + mutable bool m_is_folder; double m_thumbnailProgress; os::Surface* m_thumbnail; #ifdef _WIN32 @@ -87,10 +104,11 @@ public: bool isFolder() const override; bool isBrowsable() const override; bool isHidden() const override; + bool isExistent() const override; - std::string keyName() const override; - std::string fileName() const override; - std::string displayName() const override; + const std::string& keyName() const override; + const std::string& fileName() const override; + const std::string& displayName() const override; IFileItem* parent() const override; const FileItemList& children() override; @@ -105,23 +123,37 @@ public: os::Surface* getThumbnail() override; void setThumbnail(os::Surface* thumbnail) override; + + // Calls "delete this" + void deleteItem() { + FileSystemModule::instance()->ItemRemoved(this); + + if (m_parent) { + auto& container = m_parent->m_children; + auto it = std::find(container.begin(), container.end(), this); + if (it != container.end()) + container.erase(it); + } + + auto it = fileitems_map->find(m_keyname); + if (it != fileitems_map->end()) + fileitems_map->erase(it); + + // Delete all children recursively + for (auto ichild : m_children) { + FileItem* child = static_cast(ichild); + child->m_parent = nullptr; + child->deleteItem(); + } + + delete this; + } }; -typedef std::map FileItemMap; - -// the root of the file-system -static FileItem* rootitem = NULL; -static FileItemMap* fileitems_map; -static unsigned int current_file_system_version = 0; - -#ifdef _WIN32 - static IMalloc* shl_imalloc = NULL; - static IShellFolder* shl_idesktop = NULL; -#endif +} // anonymous namespace // A more easy PIDLs interface (without using the SH* & IL* routines of W2K) #ifdef _WIN32 - static bool is_sfgaof_folder(SFGAOF attrib); static void update_by_pidl(FileItem* fileitem, SFGAOF attrib); static LPITEMIDLIST concat_pidl(LPITEMIDLIST pidlHead, LPITEMIDLIST pidlTail); static UINT get_pidl_size(LPITEMIDLIST pidl); @@ -141,7 +173,7 @@ static unsigned int current_file_system_version = 0; static void put_fileitem(FileItem* fileitem); #endif -FileSystemModule* FileSystemModule::m_instance = NULL; +FileSystemModule* FileSystemModule::m_instance = nullptr; FileSystemModule::FileSystemModule() { @@ -174,8 +206,7 @@ FileSystemModule::~FileSystemModule() { ASSERT(m_instance == this); - for (FileItemMap::iterator - it=fileitems_map->begin(); it!=fileitems_map->end(); ++it) { + for (auto it=fileitems_map->begin(); it!=fileitems_map->end(); ++it) { delete it->second; } fileitems_map->clear(); @@ -220,8 +251,8 @@ IFileItem* FileSystemModule::getRootFileItem() { // get the desktop PIDL LPITEMIDLIST pidl = NULL; - - if (SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl) != S_OK) { + HRESULT hr = SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl); + if (hr != S_OK) { // TODO do something better ASSERT(false); exit(1); @@ -261,6 +292,7 @@ IFileItem* FileSystemModule::getFileItemFromPath(const std::string& path) LPITEMIDLIST fullpidl = NULL; SFGAOF attrib = SFGAO_FOLDER; + // Default folder is desktop folder (the root item in the hierarchy) if (path.empty()) { fileitem = getRootFileItem(); //LOG("FS: > %p (root)\n", fileitem); @@ -280,8 +312,14 @@ IFileItem* FileSystemModule::getFileItemFromPath(const std::string& path) } #else { - std::string buf = remove_backslash_if_needed(path); - fileitem = get_fileitem_by_path(buf, true); + // The default folder is the user home folder + if (path.empty()) { + fileitem = get_fileitem_by_path(base::get_user_docs_folder(), true); + } + else { + std::string buf = remove_backslash_if_needed(path); + fileitem = get_fileitem_by_path(buf, true); + } } #endif @@ -317,21 +355,49 @@ bool FileItem::isHidden() const #endif } -std::string FileItem::keyName() const +bool FileItem::isExistent() const +{ + const std::string& fn = fileName(); + +#ifdef _WIN32 + if (!fn.empty() && fn.front() == ':') { // It's a PIDL of a special location + FS_TRACE("FS: isExistent() %s -> PIDL exists\n", fn.c_str()); + return true; + } +#endif + + bool result = false; + + if (base::is_directory(fn)) { + result = true; + if (!m_is_folder) + m_is_folder = true; // Update the "is folder" flag + } + else if (base::is_file(fn)) { + result = true; + if (m_is_folder) + m_is_folder = false; + } + + FS_TRACE("FS: isExistent() %s -> %s\n", fn.c_str(), (result ? "exists": "DOESN'T EXIST")); + return result; +} + +const std::string& FileItem::keyName() const { ASSERT(m_keyname != NOTINITIALIZED); return m_keyname; } -std::string FileItem::fileName() const +const std::string& FileItem::fileName() const { ASSERT(m_filename != NOTINITIALIZED); return m_filename; } -std::string FileItem::displayName() const +const std::string& FileItem::displayName() const { ASSERT(m_displayname != NOTINITIALIZED); @@ -479,20 +545,19 @@ const FileItemList& FileItem::children() } closedir(dir); } - } + } #endif // check old file-items (maybe removed directories or file-items) for (it=m_children.begin(); it!=m_children.end(); ) { child = static_cast(*it); - ASSERT(child != NULL); + ASSERT(child); if (child && child->m_removed) { it = m_children.erase(it); - - fileitems_map->erase(fileitems_map->find(child->m_keyname)); - delete child; + child->m_parent = nullptr; + child->deleteItem(); } else ++it; @@ -635,7 +700,7 @@ static void update_by_pidl(FileItem* fileitem, SFGAOF attrib) // Get the file name - if (pFolder != NULL && + if (pFolder && pFolder->GetDisplayNameOf(fileitem->m_pidl, SHGDN_NORMAL | SHGDN_FORPARSING, &strret) == S_OK) { @@ -676,7 +741,7 @@ static void update_by_pidl(FileItem* fileitem, SFGAOF attrib) fileitem->m_displayname = base::get_file_name(fileitem->m_filename); } - if (pFolder != NULL && pFolder != shl_idesktop) { + if (pFolder && pFolder != shl_idesktop) { pFolder->Release(); } } @@ -841,20 +906,32 @@ static std::string get_key_for_pidl(LPITEMIDLIST pidl) static FileItem* get_fileitem_by_fullpidl(LPITEMIDLIST fullpidl, bool create_if_not) { - FileItemMap::iterator it = fileitems_map->find(get_key_for_pidl(fullpidl)); - if (it != fileitems_map->end()) - return it->second; + auto key = get_key_for_pidl(fullpidl); + auto it = fileitems_map->find(key); + if (it != fileitems_map->end()) { + FileItem* item = it->second; + if (item->isExistent()) + return item; + else { + item->deleteItem(); + return nullptr; + } + } if (!create_if_not) - return NULL; + return nullptr; + + // Check if the pidl exists + SFGAOF attrib = SFGAO_FOLDER | SFGAO_VALIDATE; + HRESULT hr = shl_idesktop->GetAttributesOf(1, (LPCITEMIDLIST*)&fullpidl, &attrib); + if (hr != S_OK) + return nullptr; // new file-item - FileItem* fileitem = new FileItem(NULL); + FileItem* fileitem = new FileItem(nullptr); fileitem->m_fullpidl = clone_pidl(fullpidl); - SFGAOF attrib = SFGAO_FOLDER; - HRESULT hr = shl_idesktop->GetAttributesOf(1, (LPCITEMIDLIST *)&fileitem->m_fullpidl, &attrib); - if (hr == S_OK) { + { LPITEMIDLIST parent_fullpidl = clone_pidl(fileitem->m_fullpidl); remove_last_pidl(parent_fullpidl); @@ -904,9 +981,17 @@ static FileItem* get_fileitem_by_path(const std::string& path, bool create_if_no if (path.empty()) return rootitem; - FileItemMap::iterator it = fileitems_map->find(get_key_for_filename(path)); - if (it != fileitems_map->end()) - return it->second; + auto key = get_key_for_filename(path); + auto it = fileitems_map->find(key); + if (it != fileitems_map->end()) { + FileItem* item = it->second; + if (item->isExistent()) + return item; + else { + item->deleteItem(); + return nullptr; + } + } if (!create_if_not) return NULL; diff --git a/src/app/file_system.h b/src/app/file_system.h index 824b790ea..133b714c9 100644 --- a/src/app/file_system.h +++ b/src/app/file_system.h @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2019 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -10,6 +11,7 @@ #include "base/mutex.h" #include "base/paths.h" +#include "obs/signal.h" #include #include @@ -46,6 +48,8 @@ namespace app { void lock() { m_mutex.lock(); } void unlock() { m_mutex.unlock(); } + obs::signal ItemRemoved; + private: base::mutex m_mutex; }; @@ -70,9 +74,13 @@ namespace app { virtual bool isBrowsable() const = 0; virtual bool isHidden() const = 0; - virtual std::string keyName() const = 0; - virtual std::string fileName() const = 0; - virtual std::string displayName() const = 0; + // Returns false if this item doesn't exist anymore (e.g. a file + // or folder that was deleted from other process). + virtual bool isExistent() const = 0; + + virtual const std::string& keyName() const = 0; + virtual const std::string& fileName() const = 0; + virtual const std::string& displayName() const = 0; virtual IFileItem* parent() const = 0; virtual const FileItemList& children() = 0; diff --git a/src/app/ui/file_selector.cpp b/src/app/ui/file_selector.cpp index 218263152..91f244623 100644 --- a/src/app/ui/file_selector.cpp +++ b/src/app/ui/file_selector.cpp @@ -25,6 +25,7 @@ #include "app/ui/skin/skin_theme.h" #include "app/widget_loader.h" #include "base/bind.h" +#include "base/clamp.h" #include "base/convert_to.h" #include "base/fs.h" #include "base/paths.h" @@ -47,6 +48,8 @@ # define MAX_PATH 4096 // TODO this is needed for Linux, is it correct? #endif +#define FILESEL_TRACE(...) // TRACE + namespace app { using namespace app::skin; @@ -63,6 +66,9 @@ public: bool is_null() const { return m_isNull; } bool is_valid() const { return !m_isNull; } + bool exists() const { + return (is_valid() && (*m_iterator)->isExistent()); + } iterator get() { ASSERT(!m_isNull); @@ -95,6 +101,46 @@ NullableIterator navigation_position; // Current position in the n // the user. It's used only in FileSelector::Open type of dialogs. std::map preferred_open_extensions; +void adjust_navigation_history(IFileItem* item) +{ + auto it = navigation_history.begin(); + const bool valid = navigation_position.is_valid(); + int pos = (valid ? int(navigation_position.get() - it): 0); + + FILESEL_TRACE("FILESEL: Removed item '%s' detected (%p)\n", + item->fileName().c_str(), item); + if (valid) { + FILESEL_TRACE("FILESEL: Old navigation pos [%d] = %s\n", + pos, (*navigation_position.get())->fileName().c_str()); + } + + while (true) { + it = std::find(it, navigation_history.end(), item); + if (it == navigation_history.end()) + break; + + FILESEL_TRACE("FILESEL: Erase navigation pos [%d] = %s\n", pos, + (*it)->fileName().c_str()); + + if (pos >= it-navigation_history.begin()) + --pos; + + it = navigation_history.erase(it); + } + + if (valid && !navigation_history.empty()) { + pos = base::clamp(pos, 0, (int)navigation_history.size()-1); + navigation_position.set(navigation_history.begin() + pos); + + FILESEL_TRACE("FILESEL: New navigation pos [%d] = %s\n", + pos, (*navigation_position.get())->fileName().c_str()); + } + else { + navigation_position.reset(); + FILESEL_TRACE("FILESEL: Without new navigation pos\n"); + } +} + std::string merge_paths(const base::paths& paths) { std::string k; @@ -344,6 +390,11 @@ bool FileSelector::show( FileSystemModule* fs = FileSystemModule::instance(); LockFS lock(fs); + // Connection used to remove items from the navigation history that + // are not found in the file system anymore. + obs::scoped_connection conn = + fs->ItemRemoved.connect(&adjust_navigation_history); + fs->refresh(); // we have to find where the user should begin to browse files (start_folder) @@ -369,7 +420,7 @@ bool FileSelector::show( if (!start_folder) start_folder = fs->getFileItemFromPath(start_folder_path); - TRACE("FILESEL: Start folder '%s' (%p)\n", start_folder_path.c_str(), start_folder); + FILESEL_TRACE("FILESEL: Start folder '%s' (%p)\n", start_folder_path.c_str(), start_folder); setMinSize(gfx::Size(ui::display_w()*9/10, ui::display_h()*9/10)); remapWindow(); @@ -754,15 +805,25 @@ void FileSelector::addInNavigationHistory(IFileItem* folder) void FileSelector::onGoBack() { if (navigation_history.size() > 1) { + // The default navigation position is at the end of the history if (navigation_position.is_null()) navigation_position.set(navigation_history.end()-1); if (navigation_position.get() != navigation_history.begin()) { - navigation_position.set(navigation_position.get()-1); + // Go back to the first existent element + do { + navigation_position.set(navigation_position.get()-1); + } while (!navigation_position.exists() && + navigation_position.get() != navigation_history.begin()); - m_navigationLocked = true; - m_fileList->setCurrentFolder(*navigation_position.get()); - m_navigationLocked = false; + if (navigation_position.exists()) { + m_navigationLocked = true; + m_fileList->setCurrentFolder(*navigation_position.get()); + m_navigationLocked = false; + } + else { + navigation_position.reset(); + } } } } @@ -770,15 +831,28 @@ void FileSelector::onGoBack() void FileSelector::onGoForward() { if (navigation_history.size() > 1) { - if (navigation_position.is_null()) - navigation_position.set(navigation_history.begin()); + // This should not happen, because the forward button should be + // disabled when the navigation position is null. + if (navigation_position.is_null()) { + ASSERT(false); + navigation_position.set(navigation_history.end()-1); + } if (navigation_position.get() != navigation_history.end()-1) { - navigation_position.set(navigation_position.get()+1); + // Go forward to the first existent element + do { + navigation_position.set(navigation_position.get()+1); + } while (!navigation_position.exists() && + navigation_position.get() != navigation_history.end()-1); - m_navigationLocked = true; - m_fileList->setCurrentFolder(*navigation_position.get()); - m_navigationLocked = false; + if (navigation_position.exists()) { + m_navigationLocked = true; + m_fileList->setCurrentFolder(*navigation_position.get()); + m_navigationLocked = false; + } + else { + navigation_position.reset(); + } } } }