From 864b2a9b3f1a89f0a23748c477e2d40d6bb6503d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Fri, 21 Aug 2026 01:32:36 -0300 Subject: [PATCH] Improve asynchronous filesystem event processing - queue and batch filesystem model updates on the main thread - process directory-tree updates and worker-affine listeners asynchronously - coalesce adjacent modified events for the same path - preserve captured file metadata across delayed event delivery - add event, path, and thread-affinity filters for listener subscriptions - configure optimized per-plugin filesystem subscriptions - make listener removal wait for active callbacks safely - handle self-removal without deadlocking - support allocation-free matching of moved source paths - synchronize project directory-tree mutations with active searches - centralize diff-view button visibility around find/replace state - improve code editor wrap-content and scrollbar sizing - add queue, listener, and filesystem-model regression tests --- include/eepp/scene/event.hpp | 2 + include/eepp/ui/models/filesystemmodel.hpp | 12 +- include/eepp/ui/tools/uidiffview.hpp | 1 + include/eepp/ui/uicodeeditor.hpp | 2 + src/eepp/ui/models/filesystemmodel.cpp | 42 ++-- src/eepp/ui/tools/uidiffview.cpp | 38 +++- src/eepp/ui/tools/uidocfindreplace.cpp | 2 +- src/eepp/ui/uicodeeditor.cpp | 24 ++- .../unit_tests/boundedeventqueue_tests.cpp | 108 ++++++++++ .../unit_tests/filesystemlistener_tests.cpp | 94 ++++++++ .../unit_tests/modeloperations_tests.cpp | 23 ++ src/tools/ecode/boundedeventqueue.hpp | 69 ++++++ src/tools/ecode/ecode.cpp | 19 +- src/tools/ecode/filesystemlistener.cpp | 202 ++++++++++++++---- src/tools/ecode/filesystemlistener.hpp | 67 +++++- src/tools/ecode/filesystemlisteneroptions.hpp | 92 ++++++++ .../autocomplete/autocompleteplugin.cpp | 17 ++ .../autocomplete/autocompleteplugin.hpp | 1 + src/tools/ecode/plugins/git/gitplugin.cpp | 11 + src/tools/ecode/plugins/git/gitplugin.hpp | 2 + src/tools/ecode/plugins/plugin.cpp | 10 + src/tools/ecode/plugins/plugin.hpp | 3 + src/tools/ecode/plugins/pluginmanager.cpp | 87 ++++++-- src/tools/ecode/plugins/pluginmanager.hpp | 4 +- src/tools/ecode/projectdirectorytree.cpp | 37 ++-- 25 files changed, 860 insertions(+), 109 deletions(-) create mode 100644 src/tests/unit_tests/boundedeventqueue_tests.cpp create mode 100644 src/tests/unit_tests/filesystemlistener_tests.cpp create mode 100644 src/tools/ecode/boundedeventqueue.hpp create mode 100644 src/tools/ecode/filesystemlisteneroptions.hpp diff --git a/include/eepp/scene/event.hpp b/include/eepp/scene/event.hpp index 3be68ae37..16997a371 100644 --- a/include/eepp/scene/event.hpp +++ b/include/eepp/scene/event.hpp @@ -132,6 +132,8 @@ class EE_API Event { OnFocusWithinLoss, OnItemsCountChange, OnApply, + OnShowFindReplace, + OnHideFindReplace, NoEvent = eeINDEX_NOT_FOUND }; diff --git a/include/eepp/ui/models/filesystemmodel.hpp b/include/eepp/ui/models/filesystemmodel.hpp index 972735f2d..ff3432402 100644 --- a/include/eepp/ui/models/filesystemmodel.hpp +++ b/include/eepp/ui/models/filesystemmodel.hpp @@ -224,6 +224,14 @@ class EE_API FileSystemModel : public Model { */ bool handleFileEvent( const FileEvent& event ); + /** + * @brief Processes a filesystem event using metadata captured when the event was received. + * + * This avoids querying the filesystem from the main thread and preserves the state observed by + * an asynchronous listener even if a later event changes the path before this event is applied. + */ + bool handleFileEvent( const FileEvent& event, const FileInfo& file ); + virtual bool isValid( const ModelIndex& index ) const override; virtual bool classModelRoleEnabled() { return true; } @@ -256,7 +264,9 @@ class EE_API FileSystemModel : public Model { size_t getFileIndex( Node* parent, const FileInfo& file, const Node* excludedNode = nullptr ); - bool handleFileEventLocked( const FileEvent& event ); + bool handleFileEventLocked( const FileEvent& event, const FileInfo* preparedFile = nullptr ); + + bool handleFileEvent( const FileEvent& event, const FileInfo* preparedFile ); void setupColumnNames( Translator* translator ); }; diff --git a/include/eepp/ui/tools/uidiffview.hpp b/include/eepp/ui/tools/uidiffview.hpp index 1dd7e4ba5..1ab43b394 100644 --- a/include/eepp/ui/tools/uidiffview.hpp +++ b/include/eepp/ui/tools/uidiffview.hpp @@ -135,6 +135,7 @@ class EE_API UIDiffView : public UIWidget, public WidgetCommandExecuter { void computeSubLineDiff( DiffLine& oldLine, DiffLine& newLine ); void updateEditorsText(); void updateButtonsText(); + void updateButtonsVisibility(); void createImageViewers(); bool loadImageDiffFromPaths( const std::string& oldFilePath, const std::string& newFilePath ); void updateImageDiffView(); diff --git a/include/eepp/ui/uicodeeditor.hpp b/include/eepp/ui/uicodeeditor.hpp index ba561bb7b..23f40cb58 100644 --- a/include/eepp/ui/uicodeeditor.hpp +++ b/include/eepp/ui/uicodeeditor.hpp @@ -869,6 +869,8 @@ class EE_API UICodeEditor : public UIWidget, public TextDocument::Client { void setDynamicTheming( bool set ); + const Tools::UIDocFindReplace* getFindReplace() const { return mFindReplace; } + protected: struct LastXOffset { TextPosition position{ 0, 0 }; diff --git a/src/eepp/ui/models/filesystemmodel.cpp b/src/eepp/ui/models/filesystemmodel.cpp index 6ad77fcb5..8d98874ef 100644 --- a/src/eepp/ui/models/filesystemmodel.cpp +++ b/src/eepp/ui/models/filesystemmodel.cpp @@ -764,12 +764,14 @@ size_t FileSystemModel::getFileIndex( Node* parent, const FileInfo& file, return pos; } -bool FileSystemModel::handleFileEventLocked( const FileEvent& event ) { +bool FileSystemModel::handleFileEventLocked( const FileEvent& event, + const FileInfo* preparedFile ) { switch ( event.type ) { case FileSystemEventType::Add: { - FileInfo file( event.directory + event.filename, false ); + FileInfo file = + preparedFile ? *preparedFile : FileInfo( event.directory + event.filename, false ); - if ( !file.exists() ) + if ( !preparedFile && !file.exists() ) return false; if ( ( getMode() == Mode::DirectoriesOnly && !file.isDirectory() ) || @@ -789,11 +791,6 @@ bool FileSystemModel::handleFileEventLocked( const FileEvent& event ) { if ( childNodeExists ) return false; - Node* childNode = parent->createChild( file.getFileName(), *this ); - - if ( childNode == nullptr || childNode->getName().empty() ) - return false; - size_t pos = getFileIndex( parent, file ); const auto& displayCfg = getDisplayConfig(); @@ -804,6 +801,15 @@ bool FileSystemModel::handleFileEventLocked( const FileEvent& event ) { if ( pos == INDEX_ALREADY_EXISTS ) return false; + // The listener can provide metadata captured on its worker thread. Construct the node + // from that snapshot instead of probing the path again on the UI thread. + Node* childNode = eeNew( Node, ( FileInfo( file ), parent, *this ) ); + + if ( childNode == nullptr || childNode->getName().empty() ) { + eeDelete( childNode ); + return false; + } + beginInsertRows( parent->index( *this, 0 ), pos, pos ); { @@ -846,7 +852,8 @@ bool FileSystemModel::handleFileEventLocked( const FileEvent& event ) { break; } case FileSystemEventType::Delete: { - FileInfo file( event.directory + event.filename, false ); + FileInfo file = + preparedFile ? *preparedFile : FileInfo( event.directory + event.filename, false ); auto* child = getNodeFromPath( file.getFilepath(), file.isDirectory(), false ); if ( !child ) @@ -922,18 +929,19 @@ bool FileSystemModel::handleFileEventLocked( const FileEvent& event ) { break; } case FileSystemEventType::Moved: { - FileInfo file( event.directory + event.filename, false ); + FileInfo file = + preparedFile ? *preparedFile : FileInfo( event.directory + event.filename, false ); const std::string oldFilePath = FileSystem::isRelativePath( event.oldFilename ) ? event.directory + event.oldFilename : event.oldFilename; - if ( !file.exists() ) + if ( !preparedFile && !file.exists() ) return false; auto* node = getNodeFromPath( oldFilePath, false, false ); if ( !node ) { return handleFileEventLocked( - { FileSystemEventType::Add, event.directory, event.filename } ); + { FileSystemEventType::Add, event.directory, event.filename }, preparedFile ); } ModelIndex index = node->index( *this, 0 ); @@ -1050,6 +1058,14 @@ void FileSystemModel::setupColumnNames( Translator* translator ) { } bool FileSystemModel::handleFileEvent( const FileEvent& event ) { + return handleFileEvent( event, nullptr ); +} + +bool FileSystemModel::handleFileEvent( const FileEvent& event, const FileInfo& file ) { + return handleFileEvent( event, &file ); +} + +bool FileSystemModel::handleFileEvent( const FileEvent& event, const FileInfo* preparedFile ) { if ( !mInitOK ) return false; @@ -1064,7 +1080,7 @@ bool FileSystemModel::handleFileEvent( const FileEvent& event ) { { Lock l( resourceMutex() ); - ret = handleFileEventLocked( event ); + ret = handleFileEventLocked( event, preparedFile ); } if ( ret ) diff --git a/src/eepp/ui/tools/uidiffview.cpp b/src/eepp/ui/tools/uidiffview.cpp index 6ff02dacf..ddb88ca1d 100644 --- a/src/eepp/ui/tools/uidiffview.cpp +++ b/src/eepp/ui/tools/uidiffview.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -378,6 +379,14 @@ UIDiffView::UIDiffView() : mLeftPlugin->registerUpdate( mLeftEditor ); } ); + for ( auto* editor : { mEditor, mLeftEditor, mRightEditor } ) { + editor->on( Event::OnSizeChange, [this]( auto ) { onAutoSize(); } ); + + editor->on( Event::OnShowFindReplace, [this]( auto ) { updateButtonsVisibility(); } ); + + editor->on( Event::OnHideFindReplace, [this]( auto ) { updateButtonsVisibility(); } ); + } + mLeftEditor->setVisible( false ); mRightEditor->setVisible( false ); mLeftEditor->setVerticalScrollBarEnabled( false ); @@ -519,7 +528,7 @@ void UIDiffView::setViewMode( ViewMode mode ) { void UIDiffView::setViewModeToggleVisible( bool visible ) { mViewModeToggleVisible = visible; - mModeToggle->setVisible( visible ); + updateButtonsVisibility(); updateModeButton(); } @@ -537,7 +546,7 @@ void UIDiffView::setCompleteView( bool complete ) { void UIDiffView::setCompleteViewToggleVisible( bool visible ) { mCompleteViewToggleVisible = visible; - mCompleteViewToggle->setVisible( visible ); + updateButtonsVisibility(); updateModeButton(); } @@ -590,9 +599,10 @@ void UIDiffView::onAutoSize() { return; if ( mEditor && mLeftEditor && !mIsImageDiff ) { - setPixelsSize( getPixelsSize().getWidth(), mViewMode == ViewMode::Unified - ? mEditor->getPixelsSize().getHeight() - : mLeftEditor->getPixelsSize().getHeight() ); + setPixelsSize( getPixelsSize().getWidth(), + std::ceil( mViewMode == ViewMode::Unified + ? mEditor->getPixelsSize().getHeight() + : mLeftEditor->getPixelsSize().getHeight() ) ); } if ( mIsImageDiff && mLeftImageViewer && mRightImageViewer && mDiffImageViewer ) { @@ -610,7 +620,7 @@ void UIDiffView::onAutoSize() { height = std::max( height, viewImageHeight( mRightImageViewer ) ); if ( displayDiffImage ) - height = std::max( height, viewImageHeight( mDiffImageViewer ) ); + height = std::ceil( std::max( height, viewImageHeight( mDiffImageViewer ) ) ); setPixelsSize( getPixelsSize().getWidth(), height ); } @@ -1281,12 +1291,26 @@ void UIDiffView::loadFromFile( const std::string& oldFilePath, const std::string loadFromStrings( oldText, newText ); } +void UIDiffView::updateButtonsVisibility() { + bool findReplaceVisible{ false }; + for ( const auto* editor : { mEditor, mLeftEditor, mRightEditor } ) { + const auto* findReplace = editor->getFindReplace(); + if ( editor->isVisible() && findReplace && findReplace->isVisible() ) { + findReplaceVisible = true; + break; + } + } + mModeToggle->setVisible( mViewModeToggleVisible && !findReplaceVisible ); + mCompleteViewToggle->setVisible( mCompleteViewToggleVisible && !findReplaceVisible && + !mIsImageDiff ); +} + void UIDiffView::updateButtonsText() { mModeToggle->setText( i18n( "diffview_side_by_side", "Side by Side" ) ); mModeToggle->setSelected( mViewMode != ViewMode::Unified ); mCompleteViewToggle->setText( i18n( "diffview_compact", "Compact" ) ); - mCompleteViewToggle->setVisible( !mIsImageDiff ); mCompleteViewToggle->setSelected( !mShowCompleteView ); + updateButtonsVisibility(); } void UIDiffView::setSyntaxColorScheme( const SyntaxColorScheme& colorScheme ) { diff --git a/src/eepp/ui/tools/uidocfindreplace.cpp b/src/eepp/ui/tools/uidocfindreplace.cpp index 1e0f1bbeb..2ecc5aa2c 100644 --- a/src/eepp/ui/tools/uidocfindreplace.cpp +++ b/src/eepp/ui/tools/uidocfindreplace.cpp @@ -107,7 +107,7 @@ const char DOC_FIND_REPLACE_CSS[] = R"css( )css"; const char DOC_FIND_REPLACE_XML[] = R"xml( - + diff --git a/src/eepp/ui/uicodeeditor.cpp b/src/eepp/ui/uicodeeditor.cpp index ea520fe44..45c539bfc 100644 --- a/src/eepp/ui/uicodeeditor.cpp +++ b/src/eepp/ui/uicodeeditor.cpp @@ -2170,8 +2170,14 @@ void UICodeEditor::updateScrollBar() { bool showHScroll = mLongestLineWidth > viewPortWidth && ( !mDocView.isWrapEnabled() || mLineWrapType == LineWrapType::LineBreakingColumn ); + bool wasVisible = mHScrollBar->isVisible(); mHScrollBar->setEnabled( showHScroll ); mHScrollBar->setVisible( showHScroll ); + + if ( wasVisible != showHScroll ) { + // Must be updated because the horizontal scrollbar affects the visible lines count + notVisibleLineCount = (Int64)getTotalVisibleLines() - (Int64)getViewPortLineCount().y; + } } mVScrollBar->setPixelsPosition( mSize.getWidth() - mVScrollBar->getPixelsSize().getWidth(), @@ -2190,6 +2196,8 @@ void UICodeEditor::updateScrollBar() { setScrollY( mScroll.y ); mUpdatingScrollBar = false; + + onAutoSize(); } void UICodeEditor::goToLine( const TextPosition& position, bool centered, bool forceExactPosition, @@ -4902,8 +4910,13 @@ void UICodeEditor::showFindReplace() { if ( !mFindReplaceEnabled ) return; - if ( nullptr == mFindReplace ) + if ( nullptr == mFindReplace ) { mFindReplace = UIDocFindReplace::New( this, mDoc ); + mFindReplace->on( Event::OnVisibleChange, [this]( auto ) { + sendCommonEvent( mFindReplace->isVisible() ? Event::OnShowFindReplace + : Event::OnHideFindReplace ); + } ); + } mFindReplace->setReplaceDisabled( mLocked ); mFindReplace->show(); @@ -5890,10 +5903,13 @@ void UICodeEditor::loadFromXmlNode( const pugi::xml_node& node ) { void UICodeEditor::onAutoSize() { if ( mHeightPolicy == SizePolicy::WrapContent ) { - auto visibleLineCount = getDocumentView().getVisibleLinesCount(); + auto visibleLineCount = getTotalVisibleLines(); Float lineHeight = getLineHeight(); - Float height = lineHeight * visibleLineCount + getPixelsPadding().Top + - getPixelsPadding().Bottom + getTotalTopSpace(); + Float height = std::ceil( lineHeight * visibleLineCount + mPaddingPx.Top + + mPaddingPx.Bottom + getTotalTopSpace() ); + if ( mHorizontalScrollBarEnabled && mHScrollBar->isVisible() ) { + height += std::ceil( mHScrollBar->getPixelsSize().getHeight() ); + } setPixelsSize( getPixelsSize().getWidth(), height ); } } diff --git a/src/tests/unit_tests/boundedeventqueue_tests.cpp b/src/tests/unit_tests/boundedeventqueue_tests.cpp new file mode 100644 index 000000000..d612f0321 --- /dev/null +++ b/src/tests/unit_tests/boundedeventqueue_tests.cpp @@ -0,0 +1,108 @@ +#include "../../tools/ecode/boundedeventqueue.hpp" +#include "utest.h" +#include +#include +#include +#include + +using namespace ecode; + +UTEST( BoundedEventQueue, schedulesOnceAndDrainsInFifoOrder ) { + BoundedEventQueue queue; + EXPECT_TRUE( queue.push( 1 ) ); + EXPECT_FALSE( queue.push( 2 ) ); + EXPECT_FALSE( queue.push( 3 ) ); + + std::vector firstDrain; + queue.popUpTo( firstDrain, 2 ); + EXPECT_EQ( firstDrain.size(), 2u ); + EXPECT_EQ( firstDrain[0], 1 ); + EXPECT_EQ( firstDrain[1], 2 ); + EXPECT_TRUE( queue.finishDrain() ); + + std::vector secondDrain; + queue.popUpTo( secondDrain, 2 ); + EXPECT_EQ( secondDrain.size(), 1u ); + EXPECT_EQ( secondDrain[0], 3 ); + EXPECT_FALSE( queue.finishDrain() ); + EXPECT_TRUE( queue.push( 4 ) ); +} + +UTEST( BoundedEventQueue, producerDuringDrainCannotLoseWakeup ) { + BoundedEventQueue queue; + EXPECT_TRUE( queue.push( 1 ) ); + + std::vector drain; + queue.popUpTo( drain, 1 ); + EXPECT_FALSE( queue.push( 2 ) ); + EXPECT_TRUE( queue.finishDrain() ); + + drain.clear(); + queue.popUpTo( drain, 1 ); + EXPECT_EQ( drain.size(), 1u ); + EXPECT_EQ( drain[0], 2 ); + EXPECT_FALSE( queue.finishDrain() ); +} + +UTEST( BoundedEventQueue, replacesOnlyMatchingLastPendingEvent ) { + BoundedEventQueue queue; + EXPECT_TRUE( queue.pushOrReplaceLast( 1, []( int, int ) { return false; } ) ); + EXPECT_FALSE( queue.pushOrReplaceLast( 2, []( int previous, int ) { return previous == 1; } ) ); + EXPECT_FALSE( queue.pushOrReplaceLast( 3, []( int, int ) { return false; } ) ); + + std::vector drain; + queue.popUpTo( drain, 4 ); + EXPECT_EQ( drain.size(), 2u ); + EXPECT_EQ( drain[0], 2 ); + EXPECT_EQ( drain[1], 3 ); + EXPECT_FALSE( queue.finishDrain() ); +} + +UTEST( BoundedEventQueue, acceptsConcurrentProducersWithoutLoss ) { + static constexpr int ProducerCount = 4; + static constexpr int EventsPerProducer = 1000; + BoundedEventQueue queue; + std::atomic scheduleCount{ 0 }; + std::vector producers; + + for ( int producer = 0; producer < ProducerCount; ++producer ) { + producers.emplace_back( [producer, &queue, &scheduleCount] { + for ( int event = 0; event < EventsPerProducer; ++event ) { + if ( queue.push( producer * EventsPerProducer + event ) ) + ++scheduleCount; + } + } ); + } + for ( auto& producer : producers ) + producer.join(); + + EXPECT_EQ( scheduleCount.load(), 1 ); + std::vector received; + do { + queue.popUpTo( received, received.size() + 37 ); + } while ( queue.finishDrain() ); + + EXPECT_EQ( received.size(), static_cast( ProducerCount * EventsPerProducer ) ); + int lastEvent[ProducerCount] = { -1, -1, -1, -1 }; + for ( int event : received ) { + const int producer = event / EventsPerProducer; + EXPECT_EQ( event, producer * EventsPerProducer + ++lastEvent[producer] ); + } + std::sort( received.begin(), received.end() ); + for ( int event = 0; event < ProducerCount * EventsPerProducer; ++event ) + EXPECT_EQ( received[event], event ); +} + +UTEST( BoundedEventQueue, clearCancelsPendingDrainState ) { + BoundedEventQueue queue; + EXPECT_TRUE( queue.push( 1 ) ); + EXPECT_FALSE( queue.push( 2 ) ); + queue.clear(); + EXPECT_TRUE( queue.push( 3 ) ); + + std::vector drain; + queue.popUpTo( drain, 4 ); + EXPECT_EQ( drain.size(), 1u ); + EXPECT_EQ( drain[0], 3 ); + EXPECT_FALSE( queue.finishDrain() ); +} diff --git a/src/tests/unit_tests/filesystemlistener_tests.cpp b/src/tests/unit_tests/filesystemlistener_tests.cpp new file mode 100644 index 000000000..45fa7921c --- /dev/null +++ b/src/tests/unit_tests/filesystemlistener_tests.cpp @@ -0,0 +1,94 @@ +#include "../../tools/ecode/filesystemlistener.hpp" +#include "utest.h" +#include +#include +#include +#include + +using namespace ecode; + +UTEST( FileSystemListenerOptions, filtersByEventTypeAndPathPrefix ) { + FileSystemListener::ListenerOptions options; + FileSystemListenerFilter filter; + filter.eventTypes = FileSystemListener::eventTypeMask( FileSystemEventType::Add ) | + FileSystemListener::eventTypeMask( FileSystemEventType::Modified ); + filter.path = "/tmp/ecode-ipc/"; + options.filters.emplace_back( std::move( filter ) ); + + EXPECT_TRUE( options.matches( FileSystemEventType::Add, "/tmp/ecode-ipc/request" ) ); + EXPECT_TRUE( options.matches( FileSystemEventType::Modified, "/tmp/ecode-ipc/request" ) ); + EXPECT_FALSE( options.matches( FileSystemEventType::Delete, "/tmp/ecode-ipc/request" ) ); + EXPECT_FALSE( options.matches( FileSystemEventType::Add, "/tmp/other/request" ) ); +} + +UTEST( FileSystemListenerOptions, matchesAnyFilterWithoutDuplicateSemantics ) { + FileSystemListener::ListenerOptions options; + FileSystemListenerFilter config; + config.eventTypes = FileSystemListener::eventTypeMask( FileSystemEventType::Modified ); + config.path = "/tmp/plugin.json"; + config.pathMatch = FileEventPathMatch::Exact; + options.filters.emplace_back( std::move( config ) ); + FileSystemListenerFilter workspace; + workspace.path = "/tmp/workspace/"; + options.filters.emplace_back( std::move( workspace ) ); + + EXPECT_TRUE( options.matches( FileSystemEventType::Modified, "/tmp/plugin.json" ) ); + EXPECT_FALSE( options.matches( FileSystemEventType::Add, "/tmp/plugin.json" ) ); + EXPECT_TRUE( options.matches( FileSystemEventType::Add, "/tmp/workspace/file.cpp" ) ); + EXPECT_FALSE( options.matches( FileSystemEventType::Modified, "/tmp/plugin.json.backup" ) ); + EXPECT_TRUE( + options.matchesJoinedPath( FileSystemEventType::Moved, "/tmp/workspace/", 0, "file.cpp" ) ); + EXPECT_TRUE( options.matchesJoinedPath( FileSystemEventType::Moved, "/tmp/workspace", '/', + "file.cpp" ) ); +} + +UTEST( FileSystemListenerOptions, defaultsToAllEventsAndPathsOnMainThread ) { + FileSystemListener::ListenerOptions options; + EXPECT_EQ( options.affinity, FileSystemListener::ThreadAffinity::Main ); + EXPECT_TRUE( options.matches( FileSystemEventType::Add, "/any/path" ) ); + EXPECT_TRUE( options.matches( FileSystemEventType::Delete, "/another/path" ) ); + EXPECT_TRUE( options.matches( FileSystemEventType::Modified, "relative/path" ) ); + EXPECT_TRUE( options.matches( FileSystemEventType::Moved, "" ) ); +} + +UTEST( FileSystemListener, removalWaitsForActiveCallback ) { + FileSystemListenerCallbackState callbackState; + std::mutex mutex; + std::condition_variable condition; + bool callbackBegan{ false }; + bool callbackStarted{ false }; + bool releaseCallback{ false }; + std::thread callbackThread( [&] { + callbackBegan = callbackState.beginCallback(); + std::unique_lock lock( mutex ); + callbackStarted = true; + condition.notify_all(); + condition.wait( lock, [&] { return releaseCallback; } ); + lock.unlock(); + callbackState.endCallback(); + } ); + { + std::unique_lock lock( mutex ); + condition.wait( lock, [&] { return callbackStarted; } ); + } + + auto removal = std::async( std::launch::async, [&] { callbackState.removeAndWait( false ); } ); + EXPECT_EQ( removal.wait_for( std::chrono::milliseconds( 20 ) ), std::future_status::timeout ); + { + std::lock_guard lock( mutex ); + releaseCallback = true; + } + condition.notify_all(); + callbackThread.join(); + removal.get(); + EXPECT_TRUE( callbackBegan ); + EXPECT_FALSE( callbackState.beginCallback() ); +} + +UTEST( FileSystemListener, callbackCanRemoveItself ) { + FileSystemListenerCallbackState callbackState; + ASSERT_TRUE( callbackState.beginCallback() ); + callbackState.removeAndWait( true ); + callbackState.endCallback(); + EXPECT_FALSE( callbackState.beginCallback() ); +} diff --git a/src/tests/unit_tests/modeloperations_tests.cpp b/src/tests/unit_tests/modeloperations_tests.cpp index ae9fa3d9c..4387d30f1 100644 --- a/src/tests/unit_tests/modeloperations_tests.cpp +++ b/src/tests/unit_tests/modeloperations_tests.cpp @@ -298,6 +298,29 @@ UTEST( FileSystemModelMove, keepsUnopenedDestinationBranchLazy ) { nullptr ); } +UTEST( FileSystemModelEvents, preparedMetadataSurvivesDelayedAddDelivery ) { + TempTree tree; + auto directory = tree.path / "directory"; + auto path = directory / "transient.txt"; + std::filesystem::create_directories( directory ); + + auto model = FileSystemModel::New( tree.path.string() ); + ASSERT_TRUE( model->getNodeFromPath( directory.string(), true ) != nullptr ); + + std::FILE* file = std::fopen( path.string().c_str(), "wb" ); + ASSERT_TRUE( file != nullptr ); + std::fclose( file ); + FileInfo preparedFile( path.string(), false ); + ASSERT_TRUE( preparedFile.exists() ); + std::filesystem::remove( path ); + + ASSERT_TRUE( + model->handleFileEvent( { FileSystemEventType::Add, + directory.string() + FileSystem::getOSSlash(), "transient.txt" }, + preparedFile ) ); + ASSERT_TRUE( model->getNodeFromPath( path.string(), false, false ) != nullptr ); +} + static ModelIndex indexOfNode( const FileSystemModel& model, const void* node, const ModelIndex& parent = {} ) { for ( Int64 row = 0; row < (Int64)model.rowCount( parent ); ++row ) { diff --git a/src/tools/ecode/boundedeventqueue.hpp b/src/tools/ecode/boundedeventqueue.hpp new file mode 100644 index 000000000..6145bc74c --- /dev/null +++ b/src/tools/ecode/boundedeventqueue.hpp @@ -0,0 +1,69 @@ +#ifndef ECODE_BOUNDEDEVENTQUEUE_HPP +#define ECODE_BOUNDEDEVENTQUEUE_HPP + +#include +#include +#include +#include + +namespace ecode { + +// Thread-safe producer queue with a single-consumer scheduling handshake. +// push() returns true exactly when the caller must schedule the consumer. +// Once a drain completes, finishDrain() either transfers responsibility to +// the next drain or atomically makes the next producer responsible for it. +template class BoundedEventQueue { + public: + bool push( T&& event ) { + std::lock_guard lock( mMutex ); + mEvents.emplace_back( std::move( event ) ); + if ( mDrainScheduled ) + return false; + mDrainScheduled = true; + return true; + } + + template bool pushOrReplaceLast( T&& event, Predicate&& shouldReplace ) { + std::lock_guard lock( mMutex ); + if ( !mEvents.empty() && shouldReplace( mEvents.back(), event ) ) { + mEvents.back() = std::move( event ); + return false; + } + mEvents.emplace_back( std::move( event ) ); + if ( mDrainScheduled ) + return false; + mDrainScheduled = true; + return true; + } + + template void popUpTo( Container& events, std::size_t maxEvents ) { + std::lock_guard lock( mMutex ); + while ( !mEvents.empty() && events.size() < maxEvents ) { + events.emplace_back( std::move( mEvents.front() ) ); + mEvents.pop_front(); + } + } + + bool finishDrain() { + std::lock_guard lock( mMutex ); + if ( !mEvents.empty() ) + return true; + mDrainScheduled = false; + return false; + } + + void clear() { + std::lock_guard lock( mMutex ); + mEvents.clear(); + mDrainScheduled = false; + } + + private: + std::mutex mMutex; + std::deque mEvents; + bool mDrainScheduled{ false }; +}; + +} // namespace ecode + +#endif // ECODE_BOUNDEDEVENTQUEUE_HPP diff --git a/src/tools/ecode/ecode.cpp b/src/tools/ecode/ecode.cpp index d0b909046..5fba64f0d 100644 --- a/src/tools/ecode/ecode.cpp +++ b/src/tools/ecode/ecode.cpp @@ -5059,12 +5059,16 @@ void App::init( InitParameters& params ) { mFileWatcher->addWatch( mPidPath, mFileSystemListener ); mFileWatcher->watch(); mPluginManager->setFileSystemListener( mFileSystemListener ); - mIpcListenerId = - mFileSystemListener->addListener( [this]( const FileEvent& fe, const FileInfo& fi ) { - if ( !( ( fe.type == FileSystemEventType::Add || - fe.type == FileSystemEventType::Modified ) && - fe.directory == mPidPath ) ) - return; + FileSystemListener::ListenerOptions ipcListenerOptions; + FileSystemListenerFilter ipcListenerFilter; + ipcListenerFilter.eventTypes = + FileSystemListener::eventTypeMask( FileSystemEventType::Add ) | + FileSystemListener::eventTypeMask( FileSystemEventType::Modified ); + ipcListenerFilter.path = mPidPath; + ipcListenerOptions.filters.emplace_back( std::move( ipcListenerFilter ) ); + ipcListenerOptions.affinity = FileSystemListener::ThreadAffinity::Worker; + mIpcListenerId = mFileSystemListener->addListener( + [this]( const FileEvent&, const FileInfo& fi ) { std::string path; FileSystem::fileGet( fi.getFilepath(), path ); String::trimInPlace( path, ' ' ); @@ -5091,7 +5095,8 @@ void App::init( InitParameters& params ) { } ); } FileSystem::fileRemove( fi.getFilepath() ); - } ); + }, + std::move( ipcListenerOptions ) ); #endif mNotificationCenter = std::make_unique( diff --git a/src/tools/ecode/filesystemlistener.cpp b/src/tools/ecode/filesystemlistener.cpp index ef95301d0..cc3d4c5a3 100644 --- a/src/tools/ecode/filesystemlistener.cpp +++ b/src/tools/ecode/filesystemlistener.cpp @@ -1,4 +1,5 @@ #include "filesystemlistener.hpp" +#include #include #include #include @@ -8,6 +9,26 @@ namespace ecode { +static thread_local const void* currentFileSystemListenerCallback{ nullptr }; + +class FileSystemListenerCallbackGuard { + public: + FileSystemListenerCallbackGuard( FileSystemListenerCallbackState& state, + const void* listener ) : + mState( state ), mPreviousCallback( currentFileSystemListenerCallback ) { + currentFileSystemListenerCallback = listener; + } + + ~FileSystemListenerCallbackGuard() { + currentFileSystemListenerCallback = mPreviousCallback; + mState.endCallback(); + } + + private: + FileSystemListenerCallbackState& mState; + const void* mPreviousCallback; +}; + std::string getFileSystemEventTypeName( FileSystemEventType action ) { switch ( action ) { case FileSystemEventType::Add: @@ -45,6 +66,17 @@ FileSystemListener::~FileSystemListener() { // block instead of an atomic flag. eeASSERT( !Engine::existsSingleton() || Engine::isMainThread() ); *mLifetime = false; + SmallVector, 8> listeners; + { + Lock l( mCbsMutex ); + for ( auto& listener : mCbs ) + listeners.emplace_back( std::move( listener.second ) ); + mCbs.clear(); + } + for ( const auto& listener : listeners ) + listener->callbackState.removeAndWait( currentFileSystemListenerCallback == + listener.get() ); + mPendingEvents.clear(); if ( auto* scene = SceneManager::instance()->getUISceneNode() ) scene->getActionManager()->removeActionsByTagFromTarget( scene, mEventActionTag ); } @@ -56,35 +88,78 @@ static inline bool endsWithSlash( const std::string& dir ) { void FileSystemListener::handleFileAction( efsw::WatchID, const std::string& dir, const std::string& filename, efsw::Action action, const std::string& oldFilename ) { - // The whole logical event (model, directory tree, open documents, - // callbacks) must run on the main thread in its original order: the model - // update used to complete synchronously before the other consequences, and - // all of them touch UI-owned state. The destructor cancels queued actions - // (tagged with mEventActionTag), so a runnable can never outlive the - // listener. - if ( !Engine::isMainThread() ) { + eeASSERT( !Engine::isMainThread() ); + FileEvent event( (FileSystemEventType)action, dir, filename, oldFilename ); + FileInfo file( ( endsWithSlash( dir ) ? dir : ( dir + FileSystem::getOSSlash() ) ) + filename, + false ); + std::shared_ptr dirTree; + { + Lock l( mDirTreeMutex ); + dirTree = mDirTree; + } + if ( dirTree && action != efsw::Actions::Modified ) + dirTree->onChange( (ProjectDirectoryTree::Action)action, file, oldFilename ); + // Worker listeners receive the event early. The queued phase is independent: + // it updates the model and documents and then dispatches main-thread listeners. + dispatchCallbacks( ThreadAffinity::Worker, event, file, file.getFilepath(), true ); + enqueueFileAction( { std::move( event ), std::move( file ), action } ); +} + +void FileSystemListener::enqueueFileAction( PendingFileAction&& event ) { + auto* scene = SceneManager::instance()->getUISceneNode(); + if ( !scene ) + return; + + const bool schedule = mPendingEvents.pushOrReplaceLast( + std::move( event ), []( const PendingFileAction& previous, const PendingFileAction& next ) { + return previous.action == efsw::Actions::Modified && + next.action == efsw::Actions::Modified && + previous.file.getFilepath() == next.file.getFilepath(); + } ); + if ( schedule ) { + scene->runOnMainThread( + [lifetime = mLifetime, this]() { + if ( lifetime->load( std::memory_order_acquire ) ) + drainFileActions(); + }, + Time::Zero, mEventActionTag ); + } +} + +void FileSystemListener::drainFileActions() { + eeASSERT( Engine::isMainThread() ); + static constexpr std::size_t MaxEventsPerDrain = 64; + SmallVector events; + mPendingEvents.popUpTo( events, MaxEventsPerDrain ); + + for ( auto& event : events ) + processFileAction( std::move( event ) ); + + if ( mPendingEvents.finishDrain() ) { if ( auto* scene = SceneManager::instance()->getUISceneNode() ) { scene->runOnMainThread( - [lifetime = mLifetime, this, dir, filename, action, oldFilename]() { - if ( !lifetime->load( std::memory_order_acquire ) ) - return; - handleFileAction( 0, dir, filename, action, oldFilename ); + [lifetime = mLifetime, this]() { + if ( lifetime->load( std::memory_order_acquire ) ) + drainFileActions(); }, Time::Zero, mEventActionTag ); - return; + } else { + mPendingEvents.clear(); } - // No scene node: cannot safely touch UI state from this thread. - return; } +} - FileInfo file( ( endsWithSlash( dir ) ? dir : ( dir + FileSystem::getOSSlash() ) ) + filename ); +void FileSystemListener::processFileAction( PendingFileAction&& pending ) { + eeASSERT( Engine::isMainThread() ); + FileEvent& event = pending.event; + const std::string& eventPath = pending.file.getFilepath(); + FileInfo file( pending.file ); + const efsw::Action action = pending.action; switch ( action ) { case efsw::Actions::Add: case efsw::Actions::Delete: case efsw::Actions::Moved: { - FileEvent event( (FileSystemEventType)action, dir, filename, oldFilename ); - if ( Log::instance() && Log::instance()->getLogLevelThreshold() == LogLevel::Debug ) { std::string txt = "DIR ( " + event.directory + " ) FILE ( " + @@ -97,14 +172,12 @@ void FileSystemListener::handleFileAction( efsw::WatchID, const std::string& dir } if ( mFileSystemModel ) - mFileSystemModel->handleFileEvent( event ); - - if ( mDirTree ) - mDirTree->onChange( (ProjectDirectoryTree::Action)action, file, oldFilename ); + mFileSystemModel->handleFileEvent( event, file ); if ( action == efsw::Actions::Moved ) { - FileInfo oldFile( FileSystem::isRelativePath( oldFilename ) ? dir + oldFilename - : oldFilename ); + FileInfo oldFile( FileSystem::isRelativePath( event.oldFilename ) + ? event.directory + event.oldFilename + : event.oldFilename ); if ( file.isLink() ) file = FileInfo( file.linksTo() ); @@ -125,11 +198,7 @@ void FileSystemListener::handleFileAction( efsw::WatchID, const std::string& dir if ( isFileOpen( file ) ) notifyChange( file ); - Lock l( mCbsMutex ); - if ( !mCbs.empty() ) { - for ( const auto& cb : mCbs ) - cb.second( event, file ); - } + dispatchCallbacks( ThreadAffinity::Main, event, file, eventPath ); break; } @@ -139,35 +208,84 @@ void FileSystemListener::handleFileAction( efsw::WatchID, const std::string& dir if ( isFileOpen( file ) ) notifyChange( file ); - Lock l( mCbsMutex ); - if ( !mCbs.empty() ) { - FileEvent event( (FileSystemEventType)action, dir, filename, oldFilename ); - for ( const auto& cb : mCbs ) - cb.second( event, file ); - } + dispatchCallbacks( ThreadAffinity::Main, event, file, eventPath ); } } } void FileSystemListener::setDirTree( const std::shared_ptr& dirTree ) { + Lock l( mDirTreeMutex ); mDirTree = dirTree; } Uint64 FileSystemListener::addListener( const FileEventFn& fn ) { + return addListener( fn, {} ); +} + +Uint64 FileSystemListener::addListener( const FileEventFn& fn, ListenerOptions options ) { Lock l( mCbsMutex ); Uint64 id = ++mLastId; - mCbs[id] = fn; + mCbs[id] = std::make_shared( fn, std::move( options ) ); return id; } -bool FileSystemListener::removeListener( const Uint64& id ) { - Lock l( mCbsMutex ); - auto it = mCbs.find( id ); - if ( it != mCbs.end() ) { - mCbs.erase( it ); - return true; +void FileSystemListener::dispatchCallbacks( ThreadAffinity affinity, const FileEvent& event, + const FileInfo& file, const std::string& eventPath, + bool resolveLinks ) { + SmallVector, 8> callbacks; + { + Lock l( mCbsMutex ); + for ( const auto& entry : mCbs ) { + const Listener& listener = *entry.second; + if ( listener.options.affinity != affinity ) + continue; + bool matches = listener.options.matches( event.type, eventPath ); + if ( !matches && event.type == FileSystemEventType::Moved && + !event.oldFilename.empty() ) { + if ( FileSystem::isRelativePath( event.oldFilename ) ) { + matches = listener.options.matchesJoinedPath( + event.type, event.directory, + endsWithSlash( event.directory ) ? 0 : FileSystem::getOSSlash()[0], + event.oldFilename ); + } else { + matches = listener.options.matches( event.type, event.oldFilename ); + } + } + if ( matches ) + callbacks.emplace_back( entry.second ); + } } - return false; + if ( callbacks.empty() ) + return; + auto invokeCallback = [&]( const std::shared_ptr& listener, + const FileInfo& callbackFile ) { + if ( !listener->callbackState.beginCallback() ) + return; + FileSystemListenerCallbackGuard guard( listener->callbackState, listener.get() ); + listener->callback( event, callbackFile ); + }; + if ( resolveLinks && file.isLink() ) { + FileInfo resolvedFile( file.linksTo() ); + for ( const auto& listener : callbacks ) + invokeCallback( listener, resolvedFile ); + return; + } + for ( const auto& listener : callbacks ) + invokeCallback( listener, file ); +} + +bool FileSystemListener::removeListener( const Uint64& id ) { + std::shared_ptr listener; + { + Lock l( mCbsMutex ); + auto it = mCbs.find( id ); + if ( it == mCbs.end() ) + return false; + listener = std::move( it->second ); + mCbs.erase( it ); + } + listener->callbackState.removeAndWait( currentFileSystemListenerCallback == listener.get() ); + return true; } bool FileSystemListener::isFileOpen( const FileInfo& file ) { diff --git a/src/tools/ecode/filesystemlistener.hpp b/src/tools/ecode/filesystemlistener.hpp index c8ef8dc24..aada6295f 100644 --- a/src/tools/ecode/filesystemlistener.hpp +++ b/src/tools/ecode/filesystemlistener.hpp @@ -1,8 +1,11 @@ #ifndef ECODE_FILESYSTEMLISTENER_HPP #define ECODE_FILESYSTEMLISTENER_HPP +#include "boundedeventqueue.hpp" +#include "filesystemlisteneroptions.hpp" #include "projectdirectorytree.hpp" #include +#include #include #include #include @@ -17,9 +20,44 @@ using namespace EE::UI::Tools; namespace ecode { +class FileSystemListenerCallbackState { + public: + bool beginCallback() { + std::lock_guard lock( mMutex ); + if ( mRemoved ) + return false; + ++mActiveCallbacks; + return true; + } + + void endCallback() { + std::lock_guard lock( mMutex ); + if ( --mActiveCallbacks == 0 ) + mCondition.notify_all(); + } + + void removeAndWait( bool calledFromThisListener ) { + std::unique_lock lock( mMutex ); + mRemoved = true; + if ( !calledFromThisListener ) + mCondition.wait( lock, [this] { return mActiveCallbacks == 0; } ); + } + + private: + std::mutex mMutex; + std::condition_variable mCondition; + std::size_t mActiveCallbacks{ 0 }; + bool mRemoved{ false }; +}; + class FileSystemListener : public efsw::FileWatchListener { public: typedef std::function FileEventFn; + using ThreadAffinity = FileEventThreadAffinity; + using ListenerOptions = FileSystemListenerOptions; + static constexpr FileEventTypeMask eventTypeMask( FileSystemEventType type ) { + return fileEventTypeMask( type ); + } FileSystemListener( UICodeEditorSplitter* codeSplitter, std::shared_ptr fileSystemModel, @@ -36,22 +74,49 @@ class FileSystemListener : public efsw::FileWatchListener { Uint64 addListener( const FileEventFn& fn ); + Uint64 addListener( const FileEventFn& fn, ListenerOptions options ); + bool removeListener( const Uint64& id ); protected: + struct PendingFileAction { + FileEvent event; + FileInfo file; + efsw::Action action; + }; + struct Listener { + FileEventFn callback; + ListenerOptions options; + FileSystemListenerCallbackState callbackState; + + Listener( FileEventFn callback, ListenerOptions options ) : + callback( std::move( callback ) ), options( std::move( options ) ) {} + }; + UICodeEditorSplitter* mSplitter; std::shared_ptr mFileSystemModel; std::shared_ptr mDirTree; std::atomic mLastId{ 0 }; - std::unordered_map mCbs; + std::unordered_map> mCbs; std::vector mIgnoredFiles; Mutex mCbsMutex; + Mutex mDirTreeMutex; + BoundedEventQueue mPendingEvents; // Tag of the queued main-thread file-event actions; the destructor cancels // them and marks the lifetime token dead so a queued runnable can never // dereference the destroyed listener. Uint64 mEventActionTag{ 0 }; std::shared_ptr> mLifetime; + void enqueueFileAction( PendingFileAction&& event ); + + void drainFileActions(); + + void processFileAction( PendingFileAction&& event ); + + void dispatchCallbacks( ThreadAffinity affinity, const FileEvent& event, const FileInfo& file, + const std::string& eventPath, bool resolveLinks = false ); + bool isFileOpen( const FileInfo& file ); void notifyChange( const FileInfo& file ); diff --git a/src/tools/ecode/filesystemlisteneroptions.hpp b/src/tools/ecode/filesystemlisteneroptions.hpp new file mode 100644 index 000000000..c6937652d --- /dev/null +++ b/src/tools/ecode/filesystemlisteneroptions.hpp @@ -0,0 +1,92 @@ +#ifndef ECODE_FILESYSTEMLISTENEROPTIONS_HPP +#define ECODE_FILESYSTEMLISTENEROPTIONS_HPP + +#include +#include + +namespace ecode { + +using namespace EE::UI::Models; +using FileEventTypeMask = EE::Uint32; + +enum class FileEventThreadAffinity { Main, Worker }; +enum class FileEventPathMatch { Prefix, Exact }; + +static constexpr FileEventTypeMask fileEventTypeMask( FileSystemEventType type ) { + return 1u << static_cast( type ); +} + +struct FileSystemListenerFilter { + FileEventTypeMask eventTypes{ 0xFFFFFFFFu }; + std::string path; + FileEventPathMatch pathMatch{ FileEventPathMatch::Prefix }; + + bool matches( FileSystemEventType type, const std::string& filePath ) const { + if ( ( eventTypes & fileEventTypeMask( type ) ) == 0 ) + return false; + if ( path.empty() ) + return true; + return pathMatch == FileEventPathMatch::Exact + ? filePath == path + : filePath.compare( 0, path.size(), path ) == 0; + } + + bool matchesJoinedPath( FileSystemEventType type, const std::string& directory, char separator, + const std::string& filename ) const { + if ( ( eventTypes & fileEventTypeMask( type ) ) == 0 ) + return false; + if ( path.empty() ) + return true; + const std::size_t joinedSize = + directory.size() + ( separator != 0 ? 1 : 0 ) + filename.size(); + if ( path.size() > joinedSize || + ( pathMatch == FileEventPathMatch::Exact && path.size() != joinedSize ) ) + return false; + + std::size_t pathPos = 0; + auto matchesPart = [&]( const char* data, std::size_t size ) { + const std::size_t count = std::min( size, path.size() - pathPos ); + if ( count != 0 && path.compare( pathPos, count, data, count ) != 0 ) + return false; + pathPos += count; + return true; + }; + if ( !matchesPart( directory.data(), directory.size() ) ) + return false; + if ( separator != 0 && pathPos < path.size() ) { + if ( !matchesPart( &separator, 1 ) ) + return false; + } + return pathPos == path.size() || matchesPart( filename.data(), filename.size() ); + } +}; + +struct FileSystemListenerOptions { + std::vector filters; + FileEventThreadAffinity affinity{ FileEventThreadAffinity::Main }; + + bool matches( FileSystemEventType type, const std::string& filePath ) const { + if ( filters.empty() ) + return true; + for ( const auto& filter : filters ) { + if ( filter.matches( type, filePath ) ) + return true; + } + return false; + } + + bool matchesJoinedPath( FileSystemEventType type, const std::string& directory, char separator, + const std::string& filename ) const { + if ( filters.empty() ) + return true; + for ( const auto& filter : filters ) { + if ( filter.matchesJoinedPath( type, directory, separator, filename ) ) + return true; + } + return false; + } +}; + +} // namespace ecode + +#endif // ECODE_FILESYSTEMLISTENEROPTIONS_HPP diff --git a/src/tools/ecode/plugins/autocomplete/autocompleteplugin.cpp b/src/tools/ecode/plugins/autocomplete/autocompleteplugin.cpp index 7d5fa1746..11b69a143 100644 --- a/src/tools/ecode/plugins/autocomplete/autocompleteplugin.cpp +++ b/src/tools/ecode/plugins/autocomplete/autocompleteplugin.cpp @@ -674,6 +674,23 @@ void AutoCompletePlugin::onFileSystemEvent( const FileEvent& ev, const FileInfo& } } +FileSystemListenerOptions AutoCompletePlugin::getFileSystemListenerOptions() const { + auto options = Plugin::getFileSystemListenerOptions(); + const auto eventTypes = fileEventTypeMask( FileSystemEventType::Add ) | + fileEventTypeMask( FileSystemEventType::Delete ) | + fileEventTypeMask( FileSystemEventType::Modified ) | + fileEventTypeMask( FileSystemEventType::Moved ); + for ( const auto* path : { &mUserSnippetsPath, &mVSCodeSnippetsPath, &mEcodeSnippetsPath } ) { + if ( path->empty() ) + continue; + FileSystemListenerFilter filter; + filter.eventTypes = eventTypes; + filter.path = *path; + options.filters.emplace_back( std::move( filter ) ); + } + return options; +} + void AutoCompletePlugin::onRegister( UICodeEditor* editor ) { registerSnippetLocatorProvider(); Lock l( mDocMutex ); diff --git a/src/tools/ecode/plugins/autocomplete/autocompleteplugin.hpp b/src/tools/ecode/plugins/autocomplete/autocompleteplugin.hpp index 2caa3a0de..1ba503f76 100644 --- a/src/tools/ecode/plugins/autocomplete/autocompleteplugin.hpp +++ b/src/tools/ecode/plugins/autocomplete/autocompleteplugin.hpp @@ -109,6 +109,7 @@ class AutoCompletePlugin : public Plugin { bool onMouseDoubleClick( UICodeEditor*, const Vector2i&, const Uint32& ) override; bool onMouseMove( UICodeEditor*, const Vector2i&, const Uint32& ) override; void onFileSystemEvent( const FileEvent&, const FileInfo& ) override; + FileSystemListenerOptions getFileSystemListenerOptions() const override; void onLoadProject( const std::string& projectFolder, const std::string& projectStatePath ) override; diff --git a/src/tools/ecode/plugins/git/gitplugin.cpp b/src/tools/ecode/plugins/git/gitplugin.cpp index 4e7d060d3..174e6c3b3 100644 --- a/src/tools/ecode/plugins/git/gitplugin.cpp +++ b/src/tools/ecode/plugins/git/gitplugin.cpp @@ -562,6 +562,17 @@ void GitPlugin::onFileSystemEvent( const FileEvent& ev, const FileInfo& file ) { updateUI(); } +FileSystemListenerOptions GitPlugin::getFileSystemListenerOptions() const { + auto options = Plugin::getFileSystemListenerOptions(); + const auto& workspace = getManager()->getWorkspaceFolder(); + if ( !workspace.empty() ) { + FileSystemListenerFilter filter; + filter.path = workspace; + options.filters.emplace_back( std::move( filter ) ); + } + return options; +} + void GitPlugin::displayTooltip( UICodeEditor* editor, const Git::Blame& blame, const Vector2f& position ) { // HACK: Gets the old font style to restore it when the tooltip is hidden diff --git a/src/tools/ecode/plugins/git/gitplugin.hpp b/src/tools/ecode/plugins/git/gitplugin.hpp index 80975442a..aa0a92da3 100644 --- a/src/tools/ecode/plugins/git/gitplugin.hpp +++ b/src/tools/ecode/plugins/git/gitplugin.hpp @@ -54,6 +54,8 @@ class GitPlugin : public PluginBase { void onFileSystemEvent( const FileEvent& ev, const FileInfo& file ) override; + FileSystemListenerOptions getFileSystemListenerOptions() const override; + void onRegister( UICodeEditor* ) override; void onUnregister( UICodeEditor* ) override; diff --git a/src/tools/ecode/plugins/plugin.cpp b/src/tools/ecode/plugins/plugin.cpp index 4c95998dc..9d8ad2965 100644 --- a/src/tools/ecode/plugins/plugin.cpp +++ b/src/tools/ecode/plugins/plugin.cpp @@ -110,6 +110,16 @@ void Plugin::onFileSystemEvent( const FileEvent& ev, const FileInfo& file ) { } } +FileSystemListenerOptions Plugin::getFileSystemListenerOptions() const { + FileSystemListenerOptions options; + FileSystemListenerFilter filter; + filter.eventTypes = fileEventTypeMask( FileSystemEventType::Modified ); + filter.path = mConfigPath; + filter.pathMatch = FileEventPathMatch::Exact; + options.filters.emplace_back( std::move( filter ) ); + return options; +} + void Plugin::setReady( Time loadTime ) { if ( mReady ) { if ( loadTime != Time::Zero ) { diff --git a/src/tools/ecode/plugins/plugin.hpp b/src/tools/ecode/plugins/plugin.hpp index 271166744..4b6b70e3b 100644 --- a/src/tools/ecode/plugins/plugin.hpp +++ b/src/tools/ecode/plugins/plugin.hpp @@ -1,6 +1,7 @@ #ifndef ECODE_PLUGIN_HPP #define ECODE_PLUGIN_HPP +#include "../filesystemlisteneroptions.hpp" #include "lsp/lspprotocol.hpp" #include #include @@ -54,6 +55,8 @@ class Plugin : public UICodeEditorPlugin { virtual void onFileSystemEvent( const FileEvent& ev, const FileInfo& file ); + virtual FileSystemListenerOptions getFileSystemListenerOptions() const; + String i18n( const std::string& key, const String& def ) const; UIIcon* findIcon( const std::string& iconName ); diff --git a/src/tools/ecode/plugins/pluginmanager.cpp b/src/tools/ecode/plugins/pluginmanager.cpp index 7eb39ad5a..853fca5b5 100644 --- a/src/tools/ecode/plugins/pluginmanager.cpp +++ b/src/tools/ecode/plugins/pluginmanager.cpp @@ -168,6 +168,10 @@ void PluginManager::setWorkspaceFolder( const std::string& workspaceFolder ) { mWorkspaceFolder = workspaceFolder; json data{ { "folder", mWorkspaceFolder } }; sendBroadcast( PluginMessageType::WorkspaceFolderChanged, PluginMessageFormat::JSON, &data ); + // Workspace-scoped plugin filters are snapshots. Re-register after plugins + // receive the workspace notification and update their own paths. + unsubscribeFileSystemListener(); + subscribeFileSystemListener(); } PluginRequestHandle PluginManager::sendRequest( PluginMessageType type, PluginMessageFormat format, @@ -295,6 +299,7 @@ void PluginManager::setMainSplitter( UISplitter* splitter ) { void PluginManager::setFileSystemListener( FileSystemListener* listener ) { if ( listener == mFileSystemListener ) return; + unsubscribeFileSystemListener(); mFileSystemListener = listener; sendBroadcast( PluginMessageType::FileSystemListenerReady, PluginMessageFormat::Empty, nullptr ); @@ -302,34 +307,80 @@ void PluginManager::setFileSystemListener( FileSystemListener* listener ) { } void PluginManager::subscribeFileSystemListener( Plugin* plugin ) { - Lock l( mPluginsFSSubsMutex ); - mPluginsFSSubs.insert( plugin ); + { + Lock l( mPluginsFSSubsMutex ); + if ( !mPluginsFSSubs.insert( plugin ).second ) + return; + } + registerFileSystemListener( plugin ); } void PluginManager::unsubscribeFileSystemListener( Plugin* plugin ) { - Lock l( mPluginsFSSubsMutex ); - mPluginsFSSubs.erase( plugin ); + Uint64 listenerId{ 0 }; + { + Lock l( mPluginsFSSubsMutex ); + mPluginsFSSubs.erase( plugin ); + auto it = mPluginFSListenerIds.find( plugin ); + if ( it != mPluginFSListenerIds.end() ) { + listenerId = it->second; + mPluginFSListenerIds.erase( it ); + } + } + if ( listenerId != 0 && mFileSystemListener ) + mFileSystemListener->removeListener( listenerId ); } void PluginManager::subscribeFileSystemListener() { - if ( mFileSystemListenerCb != 0 || mFileSystemListener == nullptr ) + if ( mFileSystemListener == nullptr ) return; - - mFileSystemListenerCb = - mFileSystemListener->addListener( [this]( const FileEvent& ev, const FileInfo& file ) { - UnorderedSet plugins; - { - Lock l( mPluginsFSSubsMutex ); - plugins = mPluginsFSSubs; - } - for ( Plugin* plugin : plugins ) - plugin->onFileSystemEvent( ev, file ); - } ); + UnorderedSet plugins; + { + Lock l( mPluginsFSSubsMutex ); + plugins = mPluginsFSSubs; + } + for ( Plugin* plugin : plugins ) + registerFileSystemListener( plugin ); } void PluginManager::unsubscribeFileSystemListener() { - if ( mFileSystemListenerCb != 0 && mFileSystemListener ) - mFileSystemListener->removeListener( mFileSystemListenerCb ); + std::vector listenerIds; + { + Lock l( mPluginsFSSubsMutex ); + listenerIds.reserve( mPluginFSListenerIds.size() ); + for ( const auto& listener : mPluginFSListenerIds ) + listenerIds.emplace_back( listener.second ); + mPluginFSListenerIds.clear(); + } + if ( mFileSystemListener ) { + for ( Uint64 listenerId : listenerIds ) + mFileSystemListener->removeListener( listenerId ); + } +} + +void PluginManager::registerFileSystemListener( Plugin* plugin ) { + if ( mFileSystemListener == nullptr ) + return; + { + Lock l( mPluginsFSSubsMutex ); + if ( mPluginsFSSubs.find( plugin ) == mPluginsFSSubs.end() || + mPluginFSListenerIds.find( plugin ) != mPluginFSListenerIds.end() ) + return; + } + const Uint64 listenerId = mFileSystemListener->addListener( + [plugin]( const FileEvent& ev, const FileInfo& file ) { + plugin->onFileSystemEvent( ev, file ); + }, + plugin->getFileSystemListenerOptions() ); + bool removeListener{ false }; + { + Lock l( mPluginsFSSubsMutex ); + if ( mPluginsFSSubs.find( plugin ) != mPluginsFSSubs.end() ) + mPluginFSListenerIds[plugin] = listenerId; + else + removeListener = true; + } + if ( removeListener ) + mFileSystemListener->removeListener( listenerId ); } void PluginManager::sendBroadcast( const PluginMessageType& notification, diff --git a/src/tools/ecode/plugins/pluginmanager.hpp b/src/tools/ecode/plugins/pluginmanager.hpp index f15b771e3..a135a3df4 100644 --- a/src/tools/ecode/plugins/pluginmanager.hpp +++ b/src/tools/ecode/plugins/pluginmanager.hpp @@ -393,8 +393,8 @@ class PluginManager { Mutex mPluginsFSSubsMutex; SubscribedPlugins mSubscribedPlugins; OnLoadFileCb mLoadFileFn; - Uint64 mFileSystemListenerCb{ 0 }; UnorderedSet mPluginsFSSubs; + UnorderedMap mPluginFSListenerIds; bool mClosing{ false }; bool mPluginReloadEnabled{ false }; bool mPluginsDisabled{ false }; @@ -410,6 +410,8 @@ class PluginManager { void subscribeFileSystemListener(); void unsubscribeFileSystemListener(); + + void registerFileSystemListener( Plugin* plugin ); }; class PluginsModel : public Model { diff --git a/src/tools/ecode/projectdirectorytree.cpp b/src/tools/ecode/projectdirectorytree.cpp index 04c01e00f..a06640d8a 100644 --- a/src/tools/ecode/projectdirectorytree.cpp +++ b/src/tools/ecode/projectdirectorytree.cpp @@ -245,6 +245,7 @@ std::shared_ptr ProjectDirectoryTree::asModel( const size_t& max, const std::vector& prependCommands, const std::string& basePath, const std::vector& skipExtensions ) const { + Lock rl( mMatchingMutex ); size_t namesSize = mNames.size(); size_t rmax = eemin( namesSize, max ); std::vector files; @@ -440,6 +441,7 @@ void ProjectDirectoryTree::tryAddFile( const FileInfo& file ) { } } if ( foundPattern ) { + Lock rl( mMatchingMutex ); Lock l( mFilesMutex ); auto exists = std::find( mFiles.begin(), mFiles.end(), file.getFilepath() ) != mFiles.end(); @@ -457,6 +459,7 @@ void ProjectDirectoryTree::addFile( const FileInfo& file ) { return; if ( mIgnoreHidden && file.isHidden() ) return; + Lock rl( mMatchingMutex ); Lock l( mFilesMutex ); std::vector files; std::vector names; @@ -490,13 +493,15 @@ void ProjectDirectoryTree::addFile( const FileInfo& file ) { } void ProjectDirectoryTree::moveFile( const FileInfo& file, const std::string& oldFilename ) { + Lock rl( mMatchingMutex ); Lock l( mFilesMutex ); if ( file.isDirectory() ) { std::string dir( file.getDirectoryPath() ); FileSystem::dirRemoveSlashAtEnd( dir ); std::string parentDir( FileSystem::fileRemoveFileName( dir ) ); FileSystem::dirAddSlashAtEnd( parentDir ); - std::string oldDir( parentDir + oldFilename ); + std::string oldDir( FileSystem::isRelativePath( oldFilename ) ? parentDir + oldFilename + : oldFilename ); FileSystem::dirAddSlashAtEnd( dir ); FileSystem::dirAddSlashAtEnd( oldDir ); std::vector files; @@ -525,7 +530,9 @@ void ProjectDirectoryTree::moveFile( const FileInfo& file, const std::string& ol } else { std::string dir( file.getDirectoryPath() ); FileSystem::dirAddSlashAtEnd( dir ); - size_t index = findFileIndex( dir + oldFilename ); + const std::string oldPath = + FileSystem::isRelativePath( oldFilename ) ? dir + oldFilename : oldFilename; + size_t index = findFileIndex( oldPath ); if ( index != std::string::npos ) { IgnoreMatcherManager matcher( getIgnoreMatcherFromPath( file.getFilepath() ) ); if ( !( mIgnoreHidden && file.isHidden() ) && @@ -554,19 +561,20 @@ void ProjectDirectoryTree::removeFile( const FileInfo& file ) { } if ( wasDir ) { - std::vector files; - std::vector names; - files.reserve( mFiles.size() ); - names.reserve( mNames.size() ); - for ( size_t i = 0; i < mFiles.size(); i++ ) { - if ( !String::startsWith( mFiles[i], removedDir ) ) { - files.emplace_back( mFiles[i] ); - names.emplace_back( mNames[i] ); - } - } - { + Lock rl( mMatchingMutex ); Lock l( mFilesMutex ); + std::vector files; + std::vector names; + files.reserve( mFiles.size() ); + names.reserve( mNames.size() ); + for ( size_t i = 0; i < mFiles.size(); i++ ) { + if ( !String::startsWith( mFiles[i], removedDir ) ) { + files.emplace_back( mFiles[i] ); + names.emplace_back( mNames[i] ); + } + } + mFiles = std::move( files ); mNames = std::move( names ); } @@ -574,9 +582,10 @@ void ProjectDirectoryTree::removeFile( const FileInfo& file ) { Lock ld2( mDirectoriesMutex ); mDirectories.erase( std::find( mDirectories.begin(), mDirectories.end(), removedDir ) ); } else { + Lock rl( mMatchingMutex ); + Lock l( mFilesMutex ); size_t index = findFileIndex( file.getFilepath() ); if ( index != std::string::npos ) { - Lock l( mFilesMutex ); mFiles.erase( mFiles.begin() + index ); mNames.erase( mNames.begin() + index ); }