diff --git a/include/eepp/system/filesystem.hpp b/include/eepp/system/filesystem.hpp index 18c31a5e5..54811d2e5 100644 --- a/include/eepp/system/filesystem.hpp +++ b/include/eepp/system/filesystem.hpp @@ -81,6 +81,11 @@ class EE_API FileSystem { /** Deletes a file from the file system. */ static bool fileRemove( const std::string& filepath ); + /** Recursively deletes a directory and all of its contents. + * @return True if the directory was removed or did not exist, false on error. + */ + static bool dirRemoveAll( const std::string& path ); + /** Moves a file or folder to the destination path * @param fromPath The path of the file or folder to move * @param toPath The folder / directory destination path diff --git a/src/eepp/system/filesystem.cpp b/src/eepp/system/filesystem.cpp index efb2c3e05..ce3980f61 100644 --- a/src/eepp/system/filesystem.cpp +++ b/src/eepp/system/filesystem.cpp @@ -202,6 +202,19 @@ bool FileSystem::fileRemove( const std::string& filepath ) { #endif } +bool FileSystem::dirRemoveAll( const std::string& path ) { +#if EE_PLATFORM == EE_PLATFORM_WIN + std::filesystem::path normalizedPath( String( path ).toWideString() ); +#else + std::filesystem::path normalizedPath( path ); +#endif + if ( normalizedPath.has_relative_path() && !normalizedPath.has_filename() ) + normalizedPath = normalizedPath.parent_path(); + std::error_code error; + std::filesystem::remove_all( normalizedPath, error ); + return !error; +} + bool FileSystem::fileHide( const std::string& filepath ) { #if EE_PLATFORM == EE_PLATFORM_WIN return SetFileAttributesW( (LPCWSTR)String( filepath ).toWideString().c_str(), diff --git a/src/tests/unit_tests/fileinfo_tests.cpp b/src/tests/unit_tests/fileinfo_tests.cpp index e442b085e..e9304c4df 100644 --- a/src/tests/unit_tests/fileinfo_tests.cpp +++ b/src/tests/unit_tests/fileinfo_tests.cpp @@ -26,13 +26,26 @@ struct TempDirectory { std::filesystem::create_directories( path ); } - ~TempDirectory() { std::filesystem::remove_all( path ); } + ~TempDirectory() { FileSystem::dirRemoveAll( path.string() ); } std::filesystem::path path; }; } // namespace +UTEST( FileSystem, dirRemoveAllHandlesTrailingSlash ) { + TempDirectory temp; + const std::filesystem::path nested = temp.path / "nested"; + std::filesystem::create_directories( nested ); + ASSERT_TRUE( FileSystem::fileWrite( ( nested / "file.txt" ).string(), "contents" ) ); + + std::string path( temp.path.string() ); + FileSystem::dirAddSlashAtEnd( path ); + EXPECT_TRUE( FileSystem::dirRemoveAll( path ) ); + EXPECT_FALSE( std::filesystem::exists( temp.path ) ); + EXPECT_TRUE( FileSystem::dirRemoveAll( path ) ); +} + UTEST( FileInfo, sameInodeUsesDeviceAndRejectsInvalidIdentity ) { FileInfoIdentity first; FileInfoIdentity same; diff --git a/src/tests/unit_tests/modeloperations_tests.cpp b/src/tests/unit_tests/modeloperations_tests.cpp index da156658b..f1d42f172 100644 --- a/src/tests/unit_tests/modeloperations_tests.cpp +++ b/src/tests/unit_tests/modeloperations_tests.cpp @@ -6,6 +6,7 @@ #include using namespace EE::UI::Models; +using namespace EE::System; namespace { @@ -131,11 +132,11 @@ struct TempTree { static unsigned long long id = 0; path = std::filesystem::temp_directory_path() / ( "eepp-filesystem-model-move-" + std::to_string( ++id ) ); - std::filesystem::remove_all( path ); + FileSystem::dirRemoveAll( path.string() ); std::filesystem::create_directories( path ); } - ~TempTree() { std::filesystem::remove_all( path ); } + ~TempTree() { FileSystem::dirRemoveAll( path.string() ); } std::filesystem::path path; }; diff --git a/src/tests/unit_tests/stringsoperations_tests.cpp b/src/tests/unit_tests/stringsoperations_tests.cpp index 23df34ca1..ff60e6802 100644 --- a/src/tests/unit_tests/stringsoperations_tests.cpp +++ b/src/tests/unit_tests/stringsoperations_tests.cpp @@ -55,10 +55,7 @@ class ScopedTestDirectory { public: explicit ScopedTestDirectory( std::filesystem::path path ) : mPath( std::move( path ) ) {} - ~ScopedTestDirectory() { - std::error_code error; - std::filesystem::remove_all( mPath, error ); - } + ~ScopedTestDirectory() { FileSystem::dirRemoveAll( mPath.string() ); } private: std::filesystem::path mPath; diff --git a/src/tools/ecode/ecode.cpp b/src/tools/ecode/ecode.cpp index de44db4fc..d0b909046 100644 --- a/src/tools/ecode/ecode.cpp +++ b/src/tools/ecode/ecode.cpp @@ -1046,14 +1046,6 @@ App::App( const size_t& jobs, const std::vector& args ) : mFontPickerController( std::make_unique( this ) ), mSettingsActions( std::make_unique( this ) ) {} -static void fsRemoveAll( const std::string& fpath ) { -#if EE_PLATFORM == EE_PLATFORM_WIN - fs::remove_all( std::filesystem::path( String( fpath ).toWideString() ) ); -#else - fs::remove_all( fpath ); -#endif -} - App::~App() { appInstance = nullptr; mDestroyingApp = true; @@ -1082,7 +1074,8 @@ App::~App() { } mDirTree.reset(); - fsRemoveAll( mPidPath ); + if ( !FileSystem::dirRemoveAll( mPidPath ) ) + Log::warning( "Failed to remove directory \"%s\"", mPidPath ); if ( mFirstInstance ) FileSystem::fileRemove( firstInstanceIndicatorPath() ); diff --git a/src/tools/ecode/settingsmenu.cpp b/src/tools/ecode/settingsmenu.cpp index 4db9c51a1..08045ed41 100644 --- a/src/tools/ecode/settingsmenu.cpp +++ b/src/tools/ecode/settingsmenu.cpp @@ -1,9 +1,6 @@ #include "settingsmenu.hpp" #include "uitreeviewfs.hpp" -#include -namespace fs = std::filesystem; - namespace ecode { String SettingsMenu::i18n( const std::string& key, const String& def ) { @@ -2866,14 +2863,6 @@ void SettingsMenu::createProjectTreeMenu() { showProjectTreeMenu(); } -static void fsRemoveAll( const std::string& fpath ) { -#if EE_PLATFORM == EE_PLATFORM_WIN - fs::remove_all( std::filesystem::path( String( fpath ).toWideString() ) ); -#else - fs::remove_all( fpath ); -#endif -} - void SettingsMenu::createProjectTreeMenu( const std::vector& files ) { if ( mProjectTreeMenu && mProjectTreeMenu->isVisible() ) mProjectTreeMenu->close(); @@ -3152,13 +3141,8 @@ void SettingsMenu::deleteFileDialog( const FileInfo& file ) { }; if ( file.isDirectory() ) { - try { - std::string fpath( file.getFilepath() ); - FileSystem::dirRemoveSlashAtEnd( fpath ); - fsRemoveAll( fpath ); - } catch ( const fs::filesystem_error& ) { + if ( !FileSystem::dirRemoveAll( file.getFilepath() ) ) errFn(); - } } else if ( !FileSystem::fileRemove( file.getFilepath() ) ) { errFn(); } diff --git a/src/tools/ecode/uitreeviewfs.cpp b/src/tools/ecode/uitreeviewfs.cpp index 107893530..230ebdd8b 100644 --- a/src/tools/ecode/uitreeviewfs.cpp +++ b/src/tools/ecode/uitreeviewfs.cpp @@ -8,8 +8,6 @@ #include #include -#include - namespace ecode { static const std::map getDefaultKeybindings() { @@ -405,14 +403,10 @@ void UITreeViewFS::deleteItems( const std::vector& paths ) { msgBox->on( Event::OnConfirm, [paths]( const Event* ) { for ( const auto& path : paths ) { FileInfo info( path ); - try { - if ( info.isDirectory() ) { - std::filesystem::remove_all( std::filesystem::path( path ) ); - } else { - FileSystem::fileRemove( path ); - } - } catch ( const std::filesystem::filesystem_error& ) { - } + if ( info.isDirectory() ) + FileSystem::dirRemoveAll( path ); + else + FileSystem::fileRemove( path ); } } ); }