Implement dynamic status update of the dictionaries

[user202729]

Summary of changes

(See also the linked issue.) In summary, this change will make it such that if some dictionaries are fast to load and others are slow to load, the ↻ icon will only display on the dictionaries which haven't finished loading.

Breaking change:

• add a new hook (and a new corresponding qt signal)
• I did some refactor, in that previously StenoLoadingManager.load() returns a list of mixed DictionaryLoaderException and JsonDictionary etc. objects, only for the Exception objects to be immediately converted into ErroredDictionary object in engine.py right after, now load() returns a list of mixed ErroredDictionary and JsonDictionary objects which are homogeneous (both are instances of StenoDictionary).

Note:

The favorite icon is not shown until all dictionaries are loaded (although it might be impossible to determine it correctly in advance before all dictionaries are loaded)

Note: make sure that there's no race condition and the final dictionaries_changed hook is always triggered later than the dictionary_state_changed hooks.

Closes #1331 .

Pull Request Checklist

• Changes have tests
• News fragment added in news.d. See documentation for details

[user202729]

Of course the additional hooks triggered/events emitted will break the tests...

Note that if there are more than one dictionaries loaded, the checking functions might need to be modified to account for the different orders of the dictionary_state_changed events (it's not deterministic)

---

Note that this creates a circular reference loop (engine -> dictionary load manager -> dictionary load operation -> ...), but there probably isn't that many of them, and Python can collect them.

[mkrnr]

Hi @user202729, thanks for the PR!

Are you still interested in this being merged? It would need to reworked quite a bit of course to resolve the conflicts.

I had a look at the implementation. I'm a fan of keeping things simple and this additional logic does add some complexity. Not sure if it's worth it in this case.

[user202729]

Given that it's net +13 lines added and at least as many of them are documentations/explanatory comments, I wouldn't say it's much extra complexity.

My motivation for having this feature is

Sometimes some dictionaries are slower to load than the others, and it would be useful to have a visual indicator to know which one is slow.

in my case, one of them is a Python dictionary and because of a bug it takes infinitely long to load it. This feature would be useful to see which one it is.

I… will take a look to see how to resolve the conflict. Doesn't help that I'm still using 4.0.0 with heavy patches, and the version I run have pretty much everything I need (upgrading would require rewriting much of the plugins)

[user202729]

Okay, rebased. (actually I just git diff > /tmp/a.diff → git checkout upstream/main → tell AI to manually apply the diff, it mostly work, the parts that it doesn't apply correctly I fix manually)

Manual testing instruction

• install plover-python-dictionary
• make sleep1.py containing import time; time.sleep(1)
• make sleep2.py similarly
• add both to Plover
• touch both files
• click the refresh button in Plover, see that the refresh icon before sleep1.py disappear one second earlier than sleep2.py

[user202729]

The failing test which is fixed in the last commit is the following.

            # Disable first dictionary.
            [
                [
                    (valid_dict_1, False),
                    (invalid_dict_1, True),
                ],
                [
                    (valid_dict_1, False, False),
                    (invalid_dict_1, True, True),
                ],
            ],

The test asserts that there is a dictionaries_loaded hook triggered. The hook is triggered at

    def _set_dictionaries(self, dictionaries):
        def dictionaries_changed(l1, l2):
            if len(l1) != len(l2):
                return True
            for d1, d2 in zip(l1, l2):
                if d1 is not d2:
                    return True
            return False

        if not dictionaries_changed(dictionaries, self._dictionaries.dicts):
            # No change.
            return
        self._dictionaries.set_dicts(dictionaries)
        self._trigger_hook(
            "dictionaries_loaded", StenoDictionaryCollection(dictionaries)
        )

which is called by

        # Start by unloading outdated dictionaries.
        self._dictionaries_manager.unload_outdated()
        self._set_dictionaries(
            [
                d
                for d in self._dictionaries.dicts
                if d.path in config_dictionaries
                and d.path in self._dictionaries_manager
            ]
        )
        # And then (re)load all dictionaries.
        dictionaries = []
        for d in self._dictionaries_manager.load(config_dictionaries.keys()):
            if isinstance(d, ErroredDictionary):
                # Only show an error if it's new.
                if d != self._dictionaries.get(d.path):
                    log.error(
                        "loading dictionary `%s` failed: %s",
                        shorten_path(d.path),
                        str(d.exception),
                    )
            d.enabled = config_dictionaries[d.path].enabled
            dictionaries.append(d)
        self._set_dictionaries(dictionaries)

Note that the comparison is d1 is not d2. Previously the code is

-            if isinstance(result, DictionaryLoaderException):
-                d = ErroredDictionary(result.path, result.exception)

which creates a new ErroredDictionary object each time this code is called. In the new code, the ErroredDictionary object is not recreated. So I think modifying this test is safe.

[mkrnr]

Just one super trivial finding. Everything else looks good to me and I also successfully tested the functionality.

[mkrnr]

Sorry, only noticed it now: the PR is missing the news file.