Keep the list of forgotten words safe, and keep the highlight where it was

The block list is now a BlockStore. The file is replaced atomically (temporary
file, fsync, rename), created readable by the owner only, picked up again when it
changes on disk, and a word whose save failed stays hidden in memory and is
written on the next attempt instead of being lost on restart. Words with line
breaks or NUL, over 256 bytes, and lists over 20000 entries are refused.

Removing candidates from the engine's list resets its cursor to the first
candidate when the removal is in front of it, so the highlight jumped while
arrowing through candidates. The filter now puts the cursor back on the
candidate it was on.

Both parts are in headers that the window and the tests include, so the tests
exercise the code that runs: tests/blockstore_test.cpp covers restarts, failed
writes, outside edits and bad input, tests/blockfilter_test.cpp runs the filter
on a real Fcitx5 candidate list.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/blockfilter.h

@@ -0,0 +1,51 @@
+#pragma once
+
+#include <algorithm>
+#include <string>
+
+#include "fcitx/candidatelist.h"
+
+namespace fcitx::m3ui {
+
+// Removes every candidate whose text the predicate rejects from the engine's
+// list. Going through the list instead of hiding items when drawing keeps the
+// labels, the selection keys and the paging in step with what is shown.
+//
+// The list resets its cursor to the first candidate when something in front of
+// it is removed, which would make the highlight jump while arrowing through the
+// candidates. The cursor is put back on the candidate it was on.
+//
+// Returns how many candidates were removed.
+template <typename IsBlocked>
+int removeBlockedCandidates(CandidateList &list, IsBlocked &&isBlocked) {
+    auto *modifiable = list.toModifiable();
+    if (!modifiable) {
+        return 0;
+    }
+    auto *cursorList = list.toBulkCursor();
+    const int oldCursor = cursorList ? cursorList->globalCursorIndex() : -1;
+
+    int removed = 0;
+    int removedBeforeCursor = 0;
+    // Backwards, so the indexes of the ones still to check do not move.
+    for (int i = modifiable->totalSize() - 1; i >= 0; i--) {
+        if (isBlocked(modifiable->candidateFromAll(i).text().toString())) {
+            modifiable->remove(i);
+            removed++;
+            if (i < oldCursor) {
+                removedBeforeCursor++;
+            }
+        }
+    }
+
+    const int total = modifiable->totalSize();
+    if (removed > 0 && cursorList && oldCursor >= 0 && total > 0) {
+        // If the candidate under the cursor itself was removed this selects
+        // the one that took its place.
+        cursorList->setGlobalCursorIndex(
+            std::clamp(oldCursor - removedBeforeCursor, 0, total - 1));
+    }
+    return removed;
+}
+
+} // namespace fcitx::m3ui

src/blockstore.h

@@ -0,0 +1,167 @@
+#pragma once
+
+#include <algorithm>
+#include <cerrno>
+#include <filesystem>
+#include <fstream>
+#include <string>
+#include <system_error>
+#include <unordered_set>
+#include <vector>
+
+#include <fcntl.h>
+#include <unistd.h>
+
+namespace fcitx::m3ui {
+
+// The words the user told the candidate window to forget, one per line in a
+// plain text file. Everything about keeping that file safe lives here:
+//
+//  - the file is replaced atomically (temporary file, fsync, rename), so a crash
+//    or a kill in the middle of a save never leaves a half written list,
+//  - it is created readable by the owner only,
+//  - a word whose save failed stays active in memory and is written on the next
+//    attempt, it is not silently dropped,
+//  - changes made to the file by someone else (another instance, a text editor)
+//    are picked up without a restart.
+class BlockStore {
+public:
+    static constexpr size_t kMaxWordBytes = 256;
+    static constexpr size_t kMaxWords = 20000;
+
+    explicit BlockStore(std::filesystem::path path) : path_(std::move(path)) {
+        load();
+    }
+
+    bool empty() const { return words_.empty(); }
+    size_t size() const { return words_.size(); }
+    bool contains(const std::string &word) const { return set_.contains(word); }
+    const std::filesystem::path &path() const { return path_; }
+
+    // Adds a word and makes sure it is on disk. The word is active in memory
+    // either way, the return value says whether it was saved.
+    bool add(const std::string &word) {
+        if (!valid(word) || words_.size() >= kMaxWords) {
+            return false;
+        }
+        reloadIfChanged();
+        if (!set_.contains(word)) {
+            insert(word);
+            dirty_ = true;
+        }
+        return dirty_ ? save() : true;
+    }
+
+    // Cheap enough for every keystroke, it is one stat call.
+    void reloadIfChanged() {
+        std::error_code ec;
+        const auto time = std::filesystem::last_write_time(path_, ec);
+        const auto size = ec ? 0 : std::filesystem::file_size(path_, ec);
+        if (ec) {
+            // No file is not a change worth reacting to unless one existed.
+            if (seen_) {
+                seen_ = false;
+            }
+            if (dirty_) {
+                save();
+            }
+            return;
+        }
+        if (seen_ && time == lastTime_ && size == lastSize_) {
+            if (dirty_) {
+                save();
+            }
+            return;
+        }
+        load();
+        if (dirty_) {
+            save();
+        }
+    }
+
+    static bool valid(const std::string &word) {
+        return !word.empty() && word.size() <= kMaxWordBytes &&
+               word.find_first_of(std::string("\n\r\0", 3)) == std::string::npos;
+    }
+
+private:
+    void insert(const std::string &word) {
+        if (set_.insert(word).second) {
+            words_.push_back(word);
+        }
+    }
+
+    // Reads the file and merges it with what is only in memory.
+    void load() {
+        std::error_code ec;
+        std::ifstream in(path_);
+        std::string line;
+        size_t count = 0;
+        while (in && count < kMaxWords && std::getline(in, line)) {
+            if (!line.empty() && line.back() == '\r') {
+                line.pop_back();
+            }
+            if (valid(line)) {
+                insert(line);
+                count++;
+            }
+        }
+        remember();
+    }
+
+    void remember() {
+        std::error_code ec;
+        const auto time = std::filesystem::last_write_time(path_, ec);
+        const auto size = ec ? 0 : std::filesystem::file_size(path_, ec);
+        seen_ = !ec;
+        lastTime_ = ec ? std::filesystem::file_time_type{} : time;
+        lastSize_ = size;
+    }
+
+    bool save() {
+        std::error_code ec;
+        std::filesystem::create_directories(path_.parent_path(), ec);
+        const auto tmp = path_.string() + ".tmp" + std::to_string(::getpid());
+        const int fd = ::open(tmp.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+        if (fd < 0) {
+            return false;
+        }
+        bool ok = true;
+        for (const auto &word : words_) {
+            const std::string line = word + "\n";
+            const char *data = line.data();
+            size_t left = line.size();
+            while (left > 0 && ok) {
+                const ssize_t n = ::write(fd, data, left);
+                if (n < 0) {
+                    if (errno == EINTR) {
+                        continue;
+                    }
+                    ok = false;
+                } else {
+                    data += n;
+                    left -= static_cast<size_t>(n);
+                }
+            }
+        }
+        ok = ok && ::fsync(fd) == 0;
+        ::close(fd);
+        if (!ok || ::rename(tmp.c_str(), path_.c_str()) != 0) {
+            ::unlink(tmp.c_str());
+            return false;
+        }
+        dirty_ = false;
+        remember();
+        return true;
+    }
+
+    std::filesystem::path path_;
+    std::vector<std::string> words_; // keeps the order of the file
+    std::unordered_set<std::string> set_;
+    bool dirty_ = false;
+    bool seen_ = false;
+    std::filesystem::file_time_type lastTime_{};
+    std::uintmax_t lastSize_ = 0;
+};
+
+} // namespace fcitx::m3ui

src/classicui.cpp

@@ -10,7 +10,6 @@
 #include <cstddef>
 #include <cstdint>
 #include <filesystem>
-#include <fstream>
 #include <functional>
 #include <map>
 #include <memory>
@@ -67,7 +66,6 @@ FCITX_DEFINE_LOG_CATEGORY(m3ui_logcategory, "m3ui");
 using AccentColorDBusType = FCITX_STRING_TO_DBUS_TYPE("(ddd)");
 
 M3UI::M3UI(Instance *instance) : instance_(instance) {
-    loadBlockedWords();
 #ifdef ENABLE_DBUS
     if (auto *dbusAddon = dbus()) {
         dbus::VariantTypeRegistry::defaultRegistry()
@@ -152,32 +150,22 @@ M3UI::M3UI(Instance *instance) : instance_(instance) {
 
 M3UI::~M3UI() {}
 
-std::filesystem::path M3UI::blockedWordsPath() const {
+std::filesystem::path M3UI::blockedWordsPath() {
     return StandardPaths::global().userDirectory(StandardPathsType::PkgData) /
            "m3ui" / "blocked-words.txt";
 }
 
-void M3UI::loadBlockedWords() {
-    blockedWords_.clear();
-    std::ifstream in(blockedWordsPath());
-    std::string line;
-    while (std::getline(in, line)) {
-        if (!line.empty()) {
-            blockedWords_.insert(line);
-        }
+bool M3UI::blockWord(const std::string &word) {
+    if (blockStore_.add(word)) {
+        return true;
     }
-}
-
-void M3UI::blockWord(const std::string &word) {
-    if (word.empty() || word.find('\n') != std::string::npos ||
-        !blockedWords_.insert(word).second) {
-        return;
+    if (BlockStore::valid(word)) {
+        CLASSICUI_ERROR() << "Could not save the blocked word to "
+                          << blockStore_.path()
+                          << ", it stays hidden until Fcitx5 restarts unless "
+                             "a later save succeeds.";
     }
-    const auto path = blockedWordsPath();
-    std::error_code ec;
-    std::filesystem::create_directories(path.parent_path(), ec);
-    std::ofstream out(path, std::ios::app);
-    out << word << '\n';
+    return false;
 }
 
 void M3UI::reloadConfig() {

src/classicui.h

@@ -14,7 +14,6 @@
 #include <string>
 #include <string_view>
 #include <unordered_map>
-#include <unordered_set>
 #include <utility>
 #include <vector>
 #include "fcitx-config/configuration.h"
@@ -31,6 +30,7 @@
 #include "fcitx/event.h"
 #include "fcitx/instance.h"
 #include "fcitx/userinterface.h"
+#include "blockstore.h"
 #include "classicui_public.h"
 #include "config.h"
 #include "plasmathemewatchdog.h"
@@ -269,17 +269,22 @@ public:
 
     // Words the user chose to forget. Forgetting in the Pinyin engine only
     // removes learned words, anything from a read only dictionary would come
-    // back, so the window also hides these.
-    bool hasBlockedWords() const { return !blockedWords_.empty(); }
+    // back, so the window also hides these. The list lives in a file, see
+    // BlockStore for how it is kept safe.
+    bool hasBlockedWords() const { return !blockStore_.empty(); }
     bool isBlocked(const std::string &word) const {
-        return blockedWords_.contains(word);
+        return blockStore_.contains(word);
     }
-    void blockWord(const std::string &word);
+    // Picks up changes made to the file behind our back and retries a save that
+    // failed earlier. One stat call.
+    void reloadBlockedWords() { blockStore_.reloadIfChanged(); }
+    // Returns false if the word could not be saved, it is hidden for this
+    // session anyway.
+    bool blockWord(const std::string &word);
 
 private:
-    void loadBlockedWords();
-    std::filesystem::path blockedWordsPath() const;
-    std::unordered_set<std::string> blockedWords_;
+    static std::filesystem::path blockedWordsPath();
+    BlockStore blockStore_{blockedWordsPath()};
 
     FCITX_ADDON_DEPENDENCY_LOADER(notificationitem, instance_->addonManager());
     FCITX_ADDON_EXPORT_FUNCTION(M3UI, labelIcon);

src/inputwindow.cpp

@@ -45,6 +45,7 @@
 #include "fcitx/text.h"
 #include "fcitx/userinterface.h"
 #include "classicui.h"
+#include "blockfilter.h"
 #include "common.h"
 #include "theme.h"
 
@@ -431,16 +432,11 @@ std::pair<int, int> InputWindow::update(InputContext *inputContext) {
         candidateList = inputPanel.candidateList();
         // Drop forgotten words, removing from the engine's list keeps labels
         // and selection keys consistent with what is shown.
+        parent_->reloadBlockedWords();
         if (candidateList && parent_->hasBlockedWords()) {
-            if (auto *modifiable = candidateList->toModifiable()) {
-                for (int i = modifiable->totalSize() - 1; i >= 0; i--) {
-                    if (parent_->isBlocked(modifiable->candidateFromAll(i)
-                                               .text()
-                                               .toString())) {
-                        modifiable->remove(i);
-                    }
-                }
-            }
+            removeBlockedCandidates(*candidateList, [this](const std::string &w) {
+                return parent_->isBlocked(w);
+            });
         }
     } else if (!inputPanel.overlayMessage().empty()) {
         auxUp = inputPanel.overlayMessage();

tests/blockfilter_test.cpp

@@ -0,0 +1,116 @@
+// Runs the real filter on a real Fcitx5 candidate list.
+//   g++ -std=c++20 -Isrc tests/blockfilter_test.cpp -o /tmp/blockfilter_test
+//   (add the flags from: pkg-config --cflags --libs Fcitx5Core)
+#include <cstdio>
+#include <memory>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include "blockfilter.h"
+#include "fcitx/candidatelist.h"
+#include "fcitx/text.h"
+
+using namespace fcitx;
+
+static int failures = 0;
+#define CHECK(cond)                                                          \
+    do {                                                                     \
+        if (!(cond)) {                                                       \
+            std::printf("FAIL line %d: %s\n", __LINE__, #cond);              \
+            failures++;                                                      \
+        }                                                                    \
+    } while (0)
+
+static std::unique_ptr<CommonCandidateList> makeList(const std::vector<std::string> &words, int pageSize) {
+    auto list = std::make_unique<CommonCandidateList>();
+    list->setPageSize(pageSize);
+    for (const auto &w : words) {
+        list->append<DisplayOnlyCandidateWord>(Text(w));
+    }
+    return list;
+}
+
+static std::vector<std::string> shown(CommonCandidateList &list) {
+    std::vector<std::string> out;
+    for (int i = 0; i < list.totalSize(); i++) {
+        out.push_back(list.candidateFromAll(i).text().toString());
+    }
+    return out;
+}
+
+int main() {
+    const std::unordered_set<std::string> blocked = {"我当时就", "你不能"};
+    auto isBlocked = [&](const std::string &w) { return blocked.contains(w); };
+
+    { // the filter is reachable through the interface the window uses
+        auto list = makeList({"我", "我当时就", "你不能", "你"}, 5);
+        CandidateList *asBase = list.get();
+        CHECK(asBase->toModifiable() != nullptr);
+        CHECK(fcitx::m3ui::removeBlockedCandidates(*asBase, isBlocked) == 2);
+        CHECK((shown(*list) == std::vector<std::string>{"我", "你"}));
+    }
+    { // nothing blocked, nothing touched
+        auto list = makeList({"a", "b", "c"}, 5);
+        CHECK(fcitx::m3ui::removeBlockedCandidates(*list, isBlocked) == 0);
+        CHECK(list->totalSize() == 3);
+    }
+    { // every candidate blocked leaves an empty but valid list
+        auto list = makeList({"我当时就", "你不能"}, 5);
+        CHECK(fcitx::m3ui::removeBlockedCandidates(*list, isBlocked) == 2);
+        CHECK(list->totalSize() == 0);
+        CHECK(list->size() == 0);
+    }
+    { // the same word more than once is removed everywhere
+        auto list = makeList({"你不能", "好", "你不能", "你不能"}, 5);
+        CHECK(fcitx::m3ui::removeBlockedCandidates(*list, isBlocked) == 3);
+        CHECK((shown(*list) == std::vector<std::string>{"好"}));
+    }
+    { // over several pages the page contents and labels stay consistent
+        std::vector<std::string> words;
+        for (int i = 0; i < 12; i++) {
+            words.push_back(i % 4 == 1 ? "你不能" : "词" + std::to_string(i));
+        }
+        auto list = makeList(words, 5);
+        const int before = list->totalSize();
+        const int removed = fcitx::m3ui::removeBlockedCandidates(*list, isBlocked);
+        CHECK(removed == 3 && list->totalSize() == before - removed);
+        for (int i = 0; i < list->totalSize(); i++) {
+            CHECK(list->candidateFromAll(i).text().toString() != "你不能");
+        }
+        // the first page is full again, with candidates from the remaining ones
+        CHECK(list->size() == 5);
+        CHECK(list->totalPages() == (list->totalSize() + 4) / 5);
+        CHECK(list->candidate(0).text().toString() == "词0");
+    }
+    { // the highlight stays on the same candidate wherever the removal was
+        for (int cursor = 0; cursor < 5; cursor++) {
+            auto list = makeList({"a", "你不能", "b", "我当时就", "c"}, 5);
+            list->setGlobalCursorIndex(cursor);
+            const std::string before = list->candidateFromAll(cursor).text().toString();
+            fcitx::m3ui::removeBlockedCandidates(*list, isBlocked);
+            const std::string after = list->candidateFromAll(list->globalCursorIndex()).text().toString();
+            if (before == "你不能" || before == "我当时就") {
+                // the one under the cursor is gone, the cursor is still valid
+                CHECK(list->globalCursorIndex() >= 0 && list->globalCursorIndex() < list->totalSize());
+            } else {
+                CHECK(after == before);
+            }
+        }
+    }
+    { // removing the last candidate while the cursor is on it
+        auto list = makeList({"a", "b", "你不能"}, 5);
+        list->setGlobalCursorIndex(2);
+        fcitx::m3ui::removeBlockedCandidates(*list, isBlocked);
+        CHECK(list->globalCursorIndex() == 1);
+    }
+    { // a list the cursor does not apply to is still filtered
+        DisplayOnlyCandidateList list;
+        list.setLayoutHint(CandidateLayoutHint::Vertical);
+        CHECK(list.toModifiable() == nullptr); // cannot be modified, left alone
+        CHECK(fcitx::m3ui::removeBlockedCandidates(list, isBlocked) == 0);
+    }
+
+    std::printf(failures ? "%d check(s) failed\n" : "blockfilter: all checks passed\n", failures);
+    return failures ? 1 : 0;
+}

tests/blockstore_test.cpp

@@ -0,0 +1,122 @@
+// Checks how the list of forgotten words survives restarts, crashes and failed
+// writes.  g++ -std=c++20 -Isrc tests/blockstore_test.cpp -o /tmp/blockstore_test
+#include <sys/stat.h>
+
+#include <chrono>
+#include <cstdio>
+#include <cstdlib>
+#include <fstream>
+#include <sstream>
+#include <thread>
+
+#include "blockstore.h"
+
+using fcitx::m3ui::BlockStore;
+namespace fs = std::filesystem;
+
+static int failures = 0;
+#define CHECK(cond)                                                          \
+    do {                                                                     \
+        if (!(cond)) {                                                       \
+            std::printf("FAIL line %d: %s\n", __LINE__, #cond);              \
+            failures++;                                                      \
+        }                                                                    \
+    } while (0)
+
+static std::string slurp(const fs::path &p) {
+    std::ifstream in(p);
+    std::stringstream ss;
+    ss << in.rdbuf();
+    return ss.str();
+}
+
+int main() {
+    char tmpl[] = "/tmp/blockstore_test.XXXXXX";
+    const fs::path dir = mkdtemp(tmpl);
+    const fs::path file = dir / "m3ui" / "blocked-words.txt";
+
+    { // words survive a restart, which is a new store on the same file
+        BlockStore first(file);
+        CHECK(first.empty());
+        CHECK(first.add("伟大实践"));
+        CHECK(first.add("你不能"));
+        CHECK(first.contains("你不能"));
+        BlockStore afterRestart(file);
+        CHECK(afterRestart.size() == 2);
+        CHECK(afterRestart.contains("伟大实践") && afterRestart.contains("你不能"));
+        CHECK(slurp(file) == "伟大实践\n你不能\n");
+    }
+    { // the file is private and no temporary file is left behind
+        struct stat st {};
+        CHECK(::stat(file.c_str(), &st) == 0);
+        CHECK((st.st_mode & 0777) == 0600);
+        int stray = 0;
+        for (const auto &e : fs::directory_iterator(file.parent_path())) {
+            stray += e.path().filename() != "blocked-words.txt";
+        }
+        CHECK(stray == 0);
+    }
+    { // duplicates are not written twice, bad words are refused
+        BlockStore s(file);
+        CHECK(s.add("你不能"));
+        CHECK(s.size() == 2);
+        CHECK(!s.add(""));
+        CHECK(!s.add("two\nlines"));
+        CHECK(!s.add(std::string("nul\0byte", 8)));
+        CHECK(!s.add(std::string(BlockStore::kMaxWordBytes + 1, 'x')));
+        CHECK(s.size() == 2);
+        CHECK(slurp(file) == "伟大实践\n你不能\n");
+    }
+    { // a hand edited file with Windows line endings and blank lines still loads
+        const fs::path edited = dir / "edited.txt";
+        std::ofstream(edited, std::ios::binary) << "甲\r\n\r\n乙\r\n";
+        BlockStore s(edited);
+        CHECK(s.size() == 2 && s.contains("甲") && s.contains("乙"));
+    }
+    { // a change made by someone else is noticed without a restart
+        BlockStore running(file);
+        CHECK(!running.contains("外部"));
+        std::this_thread::sleep_for(std::chrono::milliseconds(20));
+        {
+            std::ofstream out(file, std::ios::app);
+            out << "外部\n";
+        }
+        running.reloadIfChanged();
+        CHECK(running.contains("外部"));
+        // adding after that keeps the outside change, nothing is overwritten
+        CHECK(running.add("新词"));
+        CHECK(slurp(file).find("外部") != std::string::npos);
+        CHECK(slurp(file).find("新词") != std::string::npos);
+    }
+    { // a failed save keeps the word active and is retried, not forgotten
+        const fs::path lockedDir = dir / "locked";
+        fs::create_directories(lockedDir);
+        const fs::path lockedFile = lockedDir / "blocked-words.txt";
+        BlockStore s(lockedFile);
+        fs::permissions(lockedDir, fs::perms::owner_read | fs::perms::owner_exec);
+        const bool saved = s.add("保留");
+        if (::geteuid() != 0) {
+            CHECK(!saved);
+        }
+        CHECK(s.contains("保留")); // still hidden for this session
+        fs::permissions(lockedDir, fs::perms::owner_all);
+        s.reloadIfChanged(); // the retry
+        CHECK(slurp(lockedFile) == "保留\n");
+        BlockStore restarted(lockedFile);
+        CHECK(restarted.contains("保留"));
+    }
+    { // a file that vanishes while running is written again
+        BlockStore s(file);
+        const auto before = s.size();
+        fs::remove(file);
+        s.reloadIfChanged();
+        CHECK(s.size() == before);
+        CHECK(s.add("再来"));
+        BlockStore again(file);
+        CHECK(again.contains("再来") && again.contains("伟大实践"));
+    }
+
+    fs::remove_all(dir);
+    std::printf(failures ? "%d check(s) failed\n" : "blockstore: all checks passed\n", failures);
+    return failures ? 1 : 0;
+}