hchunhui / librime-lua

Extending RIME with Lua scripts
BSD 3-Clause "New" or "Revised" License
359 stars 45 forks source link

`commit_notifier` gets lost after fcitx5's autosave #299

Closed bczhc closed 11 months ago

bczhc commented 11 months ago

Saying now I want to make a commit logger using rime-lua, then write a processor like this:

local init_flag = false

return {
    func = function(_, _)
        return kNoop
    end,
    init = function(env)
        if not init_flag then
            init_flag = true
            env.engine.context.commit_notifier:connect(function(ctx)
                local commit_text = ctx:get_commit_text()
                my_log_on_commit(commit_text)
            end)
        end
    end
}

On fcitx5's initial startup, this works fine. But fcitx5 will do automatic syncing^1, and after an "autosave", the commit_notifier will be no longer registered. After the autosave, init_flag is still true, so the notifier won't be connected again. As a result, when I start typing then, the commit logger doesn't work anymore.

However, if I don't use an init flag, and change the code as this:

return {
    func = function(_, _)
        return kNoop
    end,
    init = function(env)
        env.engine.context.commit_notifier:connect(function(ctx)
            local commit_text = ctx:get_commit_text()
            my_log_on_commit(commit_text)
        end)
    end
}

the commit notifier will be registered twice, resulting in a redundant log event after an autosave:

I20231226 20:37:37.494642 907255 types.cc:1334] [bczhc log] Commit(text=测试)
I20231226 20:37:37.494647 907255 types.cc:1334] [bczhc log] Commit(text=测试)

Steps to reproduce

  1. Create a new rime processor as above
  2. Type some text to ensure the "on commit" logger works
  3. Hide the IME (for me it's Super+space), because fcitx5 may only auto sync when the user hides their IME (idel state)
  4. Wait until fcitx5 does an autosave (by default it's 30min; see Notes below)
  5. After that, retype some text and observe the result

Result

The two cases I mentioned above.

Notes

You can modify the code of fcitx5 and shrink the autosave interval from 30min to ~30 secs.

diff --git a/src/lib/fcitx/globalconfig.cpp b/src/lib/fcitx/globalconfig.cpp
index 96b6eadc..76fe9307 100644
--- a/src/lib/fcitx/globalconfig.cpp
+++ b/src/lib/fcitx/globalconfig.cpp
@@ -190,7 +190,7 @@ FCITX_CONFIGURATION(
         autoSavePeriod{this,
                        "AutoSavePeriod",
                        _("Interval of saving user data in minutes"),
-                       30,
+                       1,
                        IntConstrain(0, 1440),
                        {},
                        {_("If value is 0, the user data may only be saved when "
diff --git a/src/lib/fcitx/instance.cpp b/src/lib/fcitx/instance.cpp
index 40d5e6ec..52925a75 100644
--- a/src/lib/fcitx/instance.cpp
+++ b/src/lib/fcitx/instance.cpp
@@ -53,8 +53,8 @@ namespace fcitx {

 namespace {

-constexpr uint64_t AutoSaveMinInUsecs = 60ull * 1000000ull; // 30 minutes
-constexpr uint64_t AutoSaveIdleTime = 60ull * 1000000ull;   // 1 minutes
+constexpr uint64_t AutoSaveMinInUsecs = 1ull * 1000000ull; // 30 minutes
+constexpr uint64_t AutoSaveIdleTime = 1ull * 1000000ull;   // 1 minutes

 FCITX_CONFIGURATION(DefaultInputMethod,
                     Option<std::vector<std::string>> defaultInputMethods{

Versions

fcitx5: 5.1.6@a4e4311 fcitx5-rime: 5.1.3@303a3fe

> Fcitx5's autosave mechanism will invoke save() for every addon every 30min

https://github.com/fcitx/fcitx5/blob/a4e4311226ead198a17248a62ac1ad05a964d43c/src/lib/fcitx/instance.cpp#L1182-L1184
shewer commented 11 months ago

the notifier needed to disconnect . fixed code

return {
    func = function(_, _)
        return kNoop
    end,
   init = function(env)
        env.notifier= env.engine.context.commit_notifier:connect(
            function(ctx)
                local commit_text = ctx:get_commit_text()
                my_log_on_commit(commit_text)
            end)
    end,
   fini(env)
      env.notifier:disconnect()
   end
}
bczhc commented 11 months ago

This works. Thanks for the help!