If the external modification is a deletion, do not ask for reloading.

The behavour when hitting "reload" on a deleted file was also confusing.

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 9eded216f1f32daa5203f5344a3b75ba3cf29bc9..2e22770f5c69ca9b3de301e93181b6140154b089 100644 (file)
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -379,15 +379,15 @@ public:
        // Make sure the file monitor monitors the good file.
        void refreshFileMonitor();
 
-       /// has it been notified of an external modification?
-       bool isExternallyModified() const { return externally_modified_; }
-
        /// Notify or clear of external modification
-       void fileExternallyModified(bool modified) const;
+       void fileExternallyModified(bool exists) const;
 
        /// Block notifications of external modifications
        FileMonitorBlocker blockFileMonitor() { return file_monitor_->block(); }
 
+       /// has been externally modified? Can be reset by the user.
+       mutable bool externally_modified_;
+
 private:
        /// So we can force access via the accessors.
        mutable Buffer const * parent_buffer;
@@ -396,9 +396,6 @@ private:
        int char_count_;
        int blank_count_;
 
-       /// has been externally modified? Can be reset by the user.
-       mutable bool externally_modified_;
-
        FileMonitorPtr file_monitor_;
 };
 
@@ -440,9 +437,8 @@ Buffer::Impl::Impl(Buffer * owner, FileName const & file, bool readonly_,
          bibfile_cache_valid_(false), cite_labels_valid_(false), preview_error_(false),
          inset(0), preview_loader_(0), cloned_buffer_(cloned_buffer),
          clone_list_(0), doing_export(false),
-         tracked_changes_present_(0), parent_buffer(0),
-         word_count_(0), char_count_(0), blank_count_(0),
-         externally_modified_(false)
+         tracked_changes_present_(0), externally_modified_(false), parent_buffer(0),
+         word_count_(0), char_count_(0), blank_count_(0)
 {
        refreshFileMonitor();
        if (!cloned_buffer_) {
@@ -5332,26 +5328,29 @@ void Buffer::Impl::refreshFileMonitor()
        // The previous file monitor is invalid
        // This also destroys the previous file monitor and all its connections
        file_monitor_ = FileSystemWatcher::monitor(filename);
-       fileExternallyModified(false);
        // file_monitor_ will be destroyed with *this, so it is not going to call a
        // destroyed object method.
-       file_monitor_->connect([this](){ fileExternallyModified(true); });
+       file_monitor_->connect([this](bool exists) {
+                       fileExternallyModified(exists);
+               });
 }
 
 
-void Buffer::Impl::fileExternallyModified(bool modified) const
+void Buffer::Impl::fileExternallyModified(bool const exists) const
 {
-       if (modified) {
-               // prevent false positives, because FileMonitorBlocker is not enough on
-               // OSX.
-               if (filename.exists() && checksum_ == filename.checksum()) {
-                       LYXERR(Debug::FILES, "External modification but "
-                              "checksum unchanged: " << filename);
-                       return;
-               }
-               lyx_clean = bak_clean = false;
+       // prevent false positives, because FileMonitorBlocker is not enough on
+       // OSX.
+       if (checksum_ == filename.checksum()) {
+               LYXERR(Debug::FILES, "External modification but "
+                      "checksum unchanged: " << filename);
+               return;
        }
-       externally_modified_ = modified;
+       lyx_clean = bak_clean = false;
+       // If the file has been deleted, only mark the file as dirty since it is
+       // pointless to prompt for reloading. If later a file is moved into this
+       // location, then the externally modified warning will appear then.
+       if (exists)
+                       externally_modified_ = true;
        if (wa_)
                wa_->updateTitles();
 }
@@ -5359,13 +5358,15 @@ void Buffer::Impl::fileExternallyModified(bool modified) const
 
 bool Buffer::notifiesExternalModification() const
 {
-       return d->isExternallyModified();
+       return d->externally_modified_;
 }
 
 
 void Buffer::clearExternalModification() const
 {
-       d->fileExternallyModified(false);
+       d->externally_modified_ = false;
+       if (d->wa_)
+               d->wa_->updateTitles();
 }
 
 

diff --git a/src/graphics/GraphicsCacheItem.cpp b/src/graphics/GraphicsCacheItem.cpp
index 576ad92bc7a8e734dcb017609ca2be0b9c7993c1..f54ce80ec3f0d790c1169745ace908d09eb1e888 100644 (file)
--- a/src/graphics/GraphicsCacheItem.cpp
+++ b/src/graphics/GraphicsCacheItem.cpp
@@ -220,7 +220,7 @@ void CacheItem::Impl::startMonitor()
                return;
        monitor_ = FileSystemWatcher::activeMonitor(filename_);
        // Disconnected at the same time as this is destroyed.
-       monitor_->connect([=](){ startLoading(); });
+       monitor_->connect([=](bool /* exists */){ startLoading(); });
 }
 
 

diff --git a/src/insets/RenderPreview.cpp b/src/insets/RenderPreview.cpp
index 75e709d577b22449ff4048795dd8970ded5d33cd..22aa45ca062fae31cff2b9f907247cd2a577d50c 100644 (file)
--- a/src/insets/RenderPreview.cpp
+++ b/src/insets/RenderPreview.cpp
@@ -304,7 +304,7 @@ void RenderMonitoredPreview::startMonitoring() const
 {
        if (!monitoring()) {
                monitor_ = FileSystemWatcher::activeMonitor(filename_);
-               monitor_->connect(changed_);
+               monitor_->connect([this](bool /* exists */){ changed_(); });
        }
 }
 

diff --git a/src/support/FileMonitor.cpp b/src/support/FileMonitor.cpp
index f1fefb8d18afb918cdc3dd676f0813b5b7a6a91e..2a794f29318b0442726d01182c11c17fb69518ec 100644 (file)
--- a/src/support/FileMonitor.cpp
+++ b/src/support/FileMonitor.cpp
@@ -118,31 +118,37 @@ FileMonitorGuard::~FileMonitorGuard()
 }
 
 
-void FileMonitorGuard::refresh()
+void FileMonitorGuard::refresh(bool const emit)
 {
        if (filename_.empty())
                return;
        QString const qfilename = toqstr(filename_);
-       if(!qwatcher_->files().contains(qfilename)) {
-               bool exists = QFile(qfilename).exists();
+       if (!qwatcher_->files().contains(qfilename)) {
+               bool const existed = exists_;
+               exists_ = QFile(qfilename).exists();
 #if (QT_VERSION >= 0x050000)
-               if (!exists || !qwatcher_->addPath(qfilename))
+               if (exists_ && !qwatcher_->addPath(qfilename))
 #else
                auto add_path = [&]() {
                        qwatcher_->addPath(qfilename);
                        return qwatcher_->files().contains(qfilename);
                };
-               if (!exists || !add_path())
+               if (exists_ && !add_path())
 #endif
                {
-                       if (exists)
-                               LYXERR(Debug::FILES,
-                                      "Could not add path to QFileSystemWatcher: "
-                                      << filename_);
-                       QTimer::singleShot(2000, this, SLOT(refresh()));
-               } else if (exists && !exists_)
-                       Q_EMIT fileChanged();
-               setExists(exists);
+                       LYXERR(Debug::FILES,
+                              "Could not add path to QFileSystemWatcher: " << filename_);
+                       QTimer::singleShot(5000, this, SLOT(refresh()));
+               } else {
+                       if (!exists_)
+                               // The standard way to overwrite a file is to delete it and
+                               // create a new file with the same name. Therefore if the file
+                               // has just been deleted, it is smart to check not too long
+                               // after whether it has been recreated.
+                           QTimer::singleShot(existed ? 100 : 2000, this, SLOT(refresh()));
+                       if (existed != exists_ && emit)
+                               Q_EMIT fileChanged(exists_);
+               }
        }
 }
 
@@ -150,11 +156,12 @@ void FileMonitorGuard::refresh()
 void FileMonitorGuard::notifyChange(QString const & path)
 {
        if (path == toqstr(filename_)) {
-               Q_EMIT fileChanged();
                // If the file has been modified by delete-move, we are notified of the
                // deletion but we no longer track the file. See
                // <https://bugreports.qt.io/browse/QTBUG-46483> (not a bug).
-               refresh();
+               // Therefore we must refresh.
+               refresh(false);
+               Q_EMIT fileChanged(exists_);
        }
 }
 
@@ -162,17 +169,15 @@ void FileMonitorGuard::notifyChange(QString const & path)
 FileMonitor::FileMonitor(std::shared_ptr<FileMonitorGuard> monitor)
        : monitor_(monitor)
 {
-       QObject::connect(monitor_.get(), SIGNAL(fileChanged()),
-                        this, SLOT(changed()));
+       connectToFileMonitorGuard();
        refresh();
 }
 
 
-void FileMonitor::reconnectToFileMonitorGuard()
+void FileMonitor::connectToFileMonitorGuard()
 {
-       monitor_->setExists(true);
-       QObject::connect(monitor_.get(), SIGNAL(fileChanged()),
-                        this, SLOT(changed()));
+       QObject::connect(monitor_.get(), SIGNAL(fileChanged(bool)),
+                        this, SLOT(changed(bool)));
 }
 
 
@@ -185,15 +190,15 @@ signals2::connection FileMonitor::connect(slot const & slot)
 void FileMonitor::disconnect()
 {
        fileChanged_.disconnect_all_slots();
-       QObject::disconnect(this, SIGNAL(fileChanged()));
+       QObject::disconnect(this, SIGNAL(fileChanged(bool)));
 }
 
 
-void FileMonitor::changed()
+void FileMonitor::changed(bool const exists)
 {
        // emit boost signal
-       fileChanged_();
-       Q_EMIT fileChanged();
+       fileChanged_(exists);
+       Q_EMIT fileChanged(exists);
 }
 
 
@@ -210,8 +215,8 @@ FileMonitorBlocker FileMonitor::block(int delay)
 FileMonitorBlockerGuard::FileMonitorBlockerGuard(FileMonitor * monitor)
        : monitor_(monitor), delay_(0)
 {
-       QObject::disconnect(monitor->monitor_.get(), SIGNAL(fileChanged()),
-                           monitor, SLOT(changed()));
+       QObject::disconnect(monitor->monitor_.get(), SIGNAL(fileChanged(bool)),
+                           monitor, SLOT(changed(bool)));
 }
 
 
@@ -229,7 +234,7 @@ FileMonitorBlockerGuard::~FileMonitorBlockerGuard()
        // from QFileSystemWatcher that we meant to ignore are not going to be
        // treated immediately, so we must yield to give us the opportunity to
        // ignore them.
-       QTimer::singleShot(delay_, monitor_, SLOT(reconnectToFileMonitorGuard()));
+       QTimer::singleShot(delay_, monitor_, SLOT(connectToFileMonitorGuard()));
 }
 
 
@@ -238,8 +243,9 @@ ActiveFileMonitor::ActiveFileMonitor(std::shared_ptr<FileMonitorGuard> monitor,
        : FileMonitor(monitor), filename_(filename), interval_(interval),
          timestamp_(0), checksum_(0), cooldown_(true)
 {
-       QObject::connect(this, SIGNAL(fileChanged()), this, SLOT(setCooldown()));
+       QObject::connect(this, SIGNAL(fileChanged(bool)), this, SLOT(setCooldown()));
        QTimer::singleShot(interval_, this, SLOT(clearCooldown()));
+       filename_.refresh();
        if (!filename_.exists())
                return;
        timestamp_ = filename_.lastModified();
@@ -254,7 +260,9 @@ void ActiveFileMonitor::checkModified()
 
        cooldown_ = true;
        bool changed = false;
-       if (!filename_.exists()) {
+       filename_.refresh();
+       bool exists = filename_.exists();
+       if (!exists) {
                changed = timestamp_ || checksum_;
                timestamp_ = 0;
                checksum_ = 0;
@@ -272,7 +280,7 @@ void ActiveFileMonitor::checkModified()
                }
        }
        if (changed)
-               FileMonitor::changed();
+               FileMonitor::changed(exists);
        QTimer::singleShot(interval_, this, SLOT(clearCooldown()));
 }
 

diff --git a/src/support/FileMonitor.h b/src/support/FileMonitor.h
index 5e516bb01283bfdb6f5d8869237c13ade3cd50cf..fdf2bca69df1f938d0833c8e9b62ac8707356707 100644 (file)
--- a/src/support/FileMonitor.h
+++ b/src/support/FileMonitor.h
@@ -108,18 +108,16 @@ public:
        ~FileMonitorGuard();
        /// absolute path being tracked
        std::string const & filename() { return filename_; }
-       /// if false, emit fileChanged() when we notice the existence of the file
-       void setExists(bool exists) { exists_ = exists; }
 
 public Q_SLOTS:
        /// Make sure it is being monitored, after e.g. a deletion. See
        /// <https://bugreports.qt.io/browse/QTBUG-46483>. This is called
        /// automatically.
-       void refresh();
+       void refresh(bool emit = true);
 
 Q_SIGNALS:
        /// Connect to this to be notified when the file changes
-       void fileChanged() const;
+       void fileChanged(bool exists) const;
 
 private Q_SLOTS:
        /// Receive notifications from the QFileSystemWatcher
@@ -128,6 +126,7 @@ private Q_SLOTS:
 private:
        std::string const filename_;
        QFileSystemWatcher * qwatcher_;
+       /// for emitting fileChanged() when the file is created or deleted
        bool exists_;
 };
 
@@ -157,7 +156,7 @@ class FileMonitor : public QObject
 public:
        FileMonitor(std::shared_ptr<FileMonitorGuard> monitor);
 
-       typedef signals2::signal<void()> sig;
+       typedef signals2::signal<void(bool)> sig;
        typedef sig::slot_type slot;
        /// Connect and you'll be informed when the file has changed.
        signals2::connection connect(slot const &);
@@ -177,7 +176,7 @@ public:
        /// inotify backend (e.g. Linux); for OSX (kqueue), a value of 100ms is
        /// unsufficient and more tests need to be done in combination with
        /// flushing/syncing to disk in order to understand how to catch one's own
-       /// operations reliably. No feedback from Windows yet. See
+       /// operations reliably. No feedback about Windows. See
        /// <https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg200252.html>.
        FileMonitorBlocker block(int delay = 0);
        /// Make sure the good file is being monitored, after e.g. a move or a
@@ -187,13 +186,13 @@ public:
 
 Q_SIGNALS:
        /// Connect to this to be notified when the file changes
-       void fileChanged() const;
+       void fileChanged(bool exists) const;
 
 protected Q_SLOTS:
        /// Receive notifications from the FileMonitorGuard
-       void changed();
+       void changed(bool exists);
        ///
-       void reconnectToFileMonitorGuard();
+       void connectToFileMonitorGuard();
 
 private:
        /// boost signal