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

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<wxFSWatchEntry> watch) = 0;
 

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
+        virtual wxDirTraverseResult OnFile(const wxString& WXUNUSED(filename))
-        // 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)
         {
-            if ( m_watcher->AddAny(wxFileName::FileName(filename),
+            // There is no need to watch individual files as we watch the
-                                   m_events, wxFSWPath_File) )
+            // parent directory which will notify us about any changes in them.
-            {
-                wxLogTrace(wxTRACE_FSWATCHER,
-                       "--- AddTree adding file '%s' ---", filename);
-            }
             return wxDIR_CONTINUE;
         }
 
         virtual wxDirTraverseResult OnDir(const wxString& dirname)
         {
-            // We can't currently watch only the files with the given filespec
+            if ( m_watcher->AddAny(wxFileName::DirName(dirname),
-            // in the subdirectories so we only watch subdirectories at all if
+                                   m_events, wxFSWPath_Tree, m_filespec) )
-            // we want to watch everything.
-            if ( m_filespec.empty() )
             {
-                if ( m_watcher->AddAny(wxFileName::DirName(dirname),
+                wxLogTrace(wxTRACE_FSWATCHER,
-                                       m_events, wxFSWPath_Dir) )
+                   "--- 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
+            m_watcher->Remove(wxFileName::DirName(dirname));
-            // 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));
-            }
-
             return wxDIR_CONTINUE;
         }
 

src/msw/fswatcher.cpp

@@ -316,8 +316,12 @@ void wxIOCPThread::ProcessNativeEvents(wxVector<wxEventProcessingData>& 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);
+            // For files, check that it matches any filespec
-            SendEvent(event);
+            if ( m_service->MatchesFilespec(path, watch->GetFilespec()) )
+            {
+                wxFileSystemWatcherEvent event(flags, path, path);
+                SendEvent(event);
+            }
         }
     }
 }

src/unix/fswatcher_inotify.cpp

@@ -282,18 +282,23 @@ protected:
             {
                 inotify_event& oldinevt = *(it2->second);
 
-                wxFileSystemWatcherEvent event(flags);
+                // Tell the owner, in case it's interested
-                if ( inevt.mask & IN_MOVED_FROM )
+                // If there's a filespec, assume he's not
+                if ( watch.GetFilespec().empty() )
                 {
-                    event.SetPath(GetEventPath(watch, inevt));
+                    wxFileSystemWatcherEvent event(flags);
-                    event.SetNewPath(GetEventPath(watch, oldinevt));
+                    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);
+            // For files, check that it matches any filespec
-            SendEvent(event);
+            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);
+            if ( watch.GetFilespec().empty() )
-            wxFileName path = GetEventPath(watch, inevt);
+            {
-            wxFileSystemWatcherEvent event(flags, path, path);
+                int flags = Native2WatcherFlags(inevt.mask);
-            SendEvent(event);
+                wxFileName path = GetEventPath(watch, inevt);
+                {
+                    wxFileSystemWatcherEvent event(flags, path, path);
+                    SendEvent(event);
+                }
+            }
 
             m_cookies.erase(it);
             delete &inevt;

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
+            // wxGTK always sets an additional watch for each subdir
-            treeitems += (subdirs*files) + subdirs + 1; // +1 for 'child'
+            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
+            // an additional watch for each subdir (+1 for the root dir itself
-            // We expect the dirs and the other 2 files to be skipped
+            // and another +1 for "child").
-            const size_t treeitems = subdirs + 1;
+            const size_t treeitems = subdirs + 2;
             m_watcher->AddTree(dir, wxFSW_EVENT_ALL, "*.txt");
 
             const int plustree = m_watcher->GetWatchedPathsCount();