From: Vadim Zeitlin Date: Mon, 15 Oct 2012 01:09:25 +0000 (+0000) Subject: Check for filespec when generating events in wxFileSystemWatcher. X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/6eef5763a81cf58ba9ca4f6adcaa996d263258a0 Check for filespec when generating events in wxFileSystemWatcher. Instead of setting watches on individual files when a non-empty filespec is given, always watch all the files but just ignore the events from the ones not matching the filespec. This makes the code simpler and fixes several bugs. See #14544. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@72681 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- diff --git a/include/wx/private/fswatcher.h b/include/wx/private/fswatcher.h index abdc8e244c..4b7a894428 100644 --- a/include/wx/private/fswatcher.h +++ b/include/wx/private/fswatcher.h @@ -89,6 +89,12 @@ public: return true; } + // Check whether any filespec matches the file's ext (if present) + bool MatchesFilespec(const wxFileName& fn, const wxString& filespec) const + { + return filespec.empty() || wxMatchWild(filespec, fn.GetFullName()); + } + protected: virtual bool DoAdd(wxSharedPtr watch) = 0; diff --git a/src/common/fswatchercmn.cpp b/src/common/fswatchercmn.cpp index b994758b60..89cb09e4a8 100644 --- a/src/common/fswatchercmn.cpp +++ b/src/common/fswatchercmn.cpp @@ -173,34 +173,20 @@ bool wxFileSystemWatcherBase::AddTree(const wxFileName& path, int events, { } - // CHECK we choose which files to delegate to Add(), maybe we should pass - // all of them to Add() and let it choose? this is useful when adding a - // file to a dir that is already watched, then not only should we know - // about that, but Add() should also behave well then - virtual wxDirTraverseResult OnFile(const wxString& filename) + virtual wxDirTraverseResult OnFile(const wxString& WXUNUSED(filename)) { - if ( m_watcher->AddAny(wxFileName::FileName(filename), - m_events, wxFSWPath_File) ) - { - wxLogTrace(wxTRACE_FSWATCHER, - "--- AddTree adding file '%s' ---", filename); - } + // There is no need to watch individual files as we watch the + // parent directory which will notify us about any changes in them. return wxDIR_CONTINUE; } virtual wxDirTraverseResult OnDir(const wxString& dirname) { - // We can't currently watch only the files with the given filespec - // in the subdirectories so we only watch subdirectories at all if - // we want to watch everything. - if ( m_filespec.empty() ) + if ( m_watcher->AddAny(wxFileName::DirName(dirname), + m_events, wxFSWPath_Tree, m_filespec) ) { - if ( m_watcher->AddAny(wxFileName::DirName(dirname), - m_events, wxFSWPath_Dir) ) - { - wxLogTrace(wxTRACE_FSWATCHER, - "--- AddTree adding directory '%s' ---", dirname); - } + wxLogTrace(wxTRACE_FSWATCHER, + "--- AddTree adding directory '%s' ---", dirname); } return wxDIR_CONTINUE; } @@ -216,7 +202,7 @@ bool wxFileSystemWatcherBase::AddTree(const wxFileName& path, int events, dir.Traverse(traverser, filespec); // Add the path itself explicitly as Traverse() doesn't return it. - AddAny(path.GetPathWithSep(), events, wxFSWPath_Dir, filespec); + AddAny(path.GetPathWithSep(), events, wxFSWPath_Tree, filespec); return true; } @@ -237,24 +223,16 @@ bool wxFileSystemWatcherBase::RemoveTree(const wxFileName& path) { } - virtual wxDirTraverseResult OnFile(const wxString& filename) + virtual wxDirTraverseResult OnFile(const wxString& WXUNUSED(filename)) { - m_watcher->Remove(wxFileName(filename)); + // We never watch the individual files when watching the tree, so + // nothing to do here. return wxDIR_CONTINUE; } virtual wxDirTraverseResult OnDir(const wxString& dirname) { - // Currently the subdirectories would have been added only if there - // is no filespec. - // - // Notice that we still need to recurse into them even if we're - // using a filespec because they can contain files matching it. - if ( m_filespec.empty() ) - { - m_watcher->Remove(wxFileName::DirName(dirname)); - } - + m_watcher->Remove(wxFileName::DirName(dirname)); return wxDIR_CONTINUE; } diff --git a/src/msw/fswatcher.cpp b/src/msw/fswatcher.cpp index 69c19e19b7..1c33ebac6e 100644 --- a/src/msw/fswatcher.cpp +++ b/src/msw/fswatcher.cpp @@ -316,8 +316,12 @@ void wxIOCPThread::ProcessNativeEvents(wxVector& events) // CHECK I heard that returned path can be either in short on long // form...need to account for that! wxFileName path = GetEventPath(*watch, e); - wxFileSystemWatcherEvent event(flags, path, path); - SendEvent(event); + // For files, check that it matches any filespec + if ( m_service->MatchesFilespec(path, watch->GetFilespec()) ) + { + wxFileSystemWatcherEvent event(flags, path, path); + SendEvent(event); + } } } } diff --git a/src/unix/fswatcher_inotify.cpp b/src/unix/fswatcher_inotify.cpp index 3b40d6aa2e..481455ad6b 100644 --- a/src/unix/fswatcher_inotify.cpp +++ b/src/unix/fswatcher_inotify.cpp @@ -282,18 +282,23 @@ protected: { inotify_event& oldinevt = *(it2->second); - wxFileSystemWatcherEvent event(flags); - if ( inevt.mask & IN_MOVED_FROM ) + // Tell the owner, in case it's interested + // If there's a filespec, assume he's not + if ( watch.GetFilespec().empty() ) { - event.SetPath(GetEventPath(watch, inevt)); - event.SetNewPath(GetEventPath(watch, oldinevt)); + wxFileSystemWatcherEvent event(flags); + if ( inevt.mask & IN_MOVED_FROM ) + { + event.SetPath(GetEventPath(watch, inevt)); + event.SetNewPath(GetEventPath(watch, oldinevt)); + } + else + { + event.SetPath(GetEventPath(watch, oldinevt)); + event.SetNewPath(GetEventPath(watch, inevt)); + } + SendEvent(event); } - else - { - event.SetPath(GetEventPath(watch, oldinevt)); - event.SetNewPath(GetEventPath(watch, inevt)); - } - SendEvent(event); m_cookies.erase(it2); delete &oldinevt; @@ -303,8 +308,12 @@ protected: else { wxFileName path = GetEventPath(watch, inevt); - wxFileSystemWatcherEvent event(flags, path, path); - SendEvent(event); + // For files, check that it matches any filespec + if ( MatchesFilespec(path, watch.GetFilespec()) ) + { + wxFileSystemWatcherEvent event(flags, path, path); + SendEvent(event); + } } } @@ -323,11 +332,18 @@ protected: wxCHECK_RET(wit != m_watchMap.end(), "Watch descriptor not present in the watch map!"); + // Tell the owner, in case it's interested + // If there's a filespec, assume he's not wxFSWatchEntry& watch = *(wit->second); - int flags = Native2WatcherFlags(inevt.mask); - wxFileName path = GetEventPath(watch, inevt); - wxFileSystemWatcherEvent event(flags, path, path); - SendEvent(event); + if ( watch.GetFilespec().empty() ) + { + int flags = Native2WatcherFlags(inevt.mask); + wxFileName path = GetEventPath(watch, inevt); + { + wxFileSystemWatcherEvent event(flags, path, path); + SendEvent(event); + } + } m_cookies.erase(it); delete &inevt; diff --git a/tests/fswatcher/fswatchertest.cpp b/tests/fswatcher/fswatchertest.cpp index 59cbfb852e..3768c349bd 100644 --- a/tests/fswatcher/fswatchertest.cpp +++ b/tests/fswatcher/fswatchertest.cpp @@ -713,8 +713,8 @@ void FileSystemWatcherTestCase::TestTrees() #ifndef __WINDOWS__ // When there's no file mask, wxMSW sets a single watch // on the trunk which is implemented recursively. - // wxGTK always sets an additional watch for each file/subdir - treeitems += (subdirs*files) + subdirs + 1; // +1 for 'child' + // wxGTK always sets an additional watch for each subdir + treeitems += subdirs + 1; // +1 for 'child' #endif // __WINDOWS__ // Store the initial count; there may already be some watches @@ -761,9 +761,9 @@ void FileSystemWatcherTestCase::TestTrees() const int initial = m_watcher->GetWatchedPathsCount(); // When we use a filter, both wxMSW and wxGTK implementations set - // an additional watch for each file/subdir. Test by passing *.txt - // We expect the dirs and the other 2 files to be skipped - const size_t treeitems = subdirs + 1; + // an additional watch for each subdir (+1 for the root dir itself + // and another +1 for "child"). + const size_t treeitems = subdirs + 2; m_watcher->AddTree(dir, wxFSW_EVENT_ALL, "*.txt"); const int plustree = m_watcher->GetWatchedPathsCount();