From cbcae469797d533b359e873d70fe610bc4c6eec0 Mon Sep 17 00:00:00 2001 From: Weng Xuetian Date: Sat, 21 Dec 2024 22:08:06 -0800 Subject: [PATCH] Fix stroke filter candidate action (#207) Filtering candidates wrongly assume candidate can only be pinyin candidate type. Use dynamic_cast to insert candidate that can be casted. --- im/pinyin/pinyin.cpp | 14 +++++- im/pinyin/pinyincandidate.cpp | 83 +++++++++++++++++++++++------------ im/pinyin/pinyincandidate.h | 31 ++++++++++--- test/CMakeLists.txt | 2 +- test/testpinyin.cpp | 48 ++++++++++++++++++-- 5 files changed, 138 insertions(+), 40 deletions(-) diff --git a/im/pinyin/pinyin.cpp b/im/pinyin/pinyin.cpp index 1743dc0..7e26223 100644 --- a/im/pinyin/pinyin.cpp +++ b/im/pinyin/pinyin.cpp @@ -1464,8 +1464,18 @@ void PinyinEngine::updateStroke(InputContext *inputContext) { } } if (strokeMatched) { - candidateList->append( - this, inputContext, candidate.text(), i); + // PinyinCandidateWord is the only forgettable. + if (dynamic_cast(&candidate)) { + candidateList->append>(this, inputContext, + candidate.text(), i); + } else if (dynamic_cast(&candidate)) { + candidateList->append>(this, inputContext, + candidate.text(), i); + } } } } diff --git a/im/pinyin/pinyincandidate.cpp b/im/pinyin/pinyincandidate.cpp index 7e3694d..ea289a1 100644 --- a/im/pinyin/pinyincandidate.cpp +++ b/im/pinyin/pinyincandidate.cpp @@ -126,53 +126,82 @@ void PinyinPunctuationCandidateWord::select(InputContext *inputContext) const { engine_->doReset(inputContext); } -StrokeFilterCandidateWord::StrokeFilterCandidateWord(PinyinEngine *engine, - InputContext *inputContext, - Text text, int index) +FilteredCandidateWord::FilteredCandidateWord(PinyinEngine *engine, + InputContext *inputContext, + Text text, int index) : engine_(engine), inputContext_(inputContext), index_(index) { setText(std::move(text)); } -std::string StrokeFilterCandidateWord::customPhraseString() const { - auto *state = inputContext_->propertyFor(&engine_->factory()); +void FilteredCandidateWord::select(InputContext *inputContext) const { + if (inputContext != inputContext_) { + return; + } + auto *state = inputContext->propertyFor(&engine_->factory()); if (!state->strokeCandidateList_ || state->strokeCandidateList_->toBulk()->totalSize() <= index_) { - return ""; + FCITX_ERROR() << "Stroke candidate is not consistent. Probably a " + "bug in implementation"; + return; } // Forward the selection to internal candidate list. - const auto &candidate = - state->strokeCandidateList_->toBulk()->candidateFromAll(index_); - return static_cast(candidate) - .customPhraseString(); + state->strokeCandidateList_->toBulk()->candidateFromAll(index_).select( + inputContext); + engine_->resetStroke(inputContext); } -size_t StrokeFilterCandidateWord::candidateIndex() const { - auto *state = inputContext_->propertyFor(&engine_->factory()); +std::string FilteredInsertableAsCustomPhrase::customPhraseString() const { + const auto *that = dynamic_cast(this); + if (!that) { + FCITX_ERROR() << "should be a subclass to FilteredCandidateWrord, bug " + "in pinyin engine."; + return ""; + } + auto *inputContext = that->inputContext(); + auto *engine = that->engine(); + auto index = that->originalIndex(); + auto *state = inputContext->propertyFor(&engine->factory()); if (!state->strokeCandidateList_ || - state->strokeCandidateList_->toBulk()->totalSize() <= index_) { - return 0; + state->strokeCandidateList_->toBulk()->totalSize() <= index) { + return ""; } // Forward the selection to internal candidate list. const auto &candidate = - state->strokeCandidateList_->toBulk()->candidateFromAll(index_); - return static_cast(candidate).candidateIndex(); + state->strokeCandidateList_->toBulk()->candidateFromAll(index); + if (const auto *insertable = + dynamic_cast(&candidate); + insertable != this) { + return insertable->customPhraseString(); + } + return ""; } -void StrokeFilterCandidateWord::select(InputContext *inputContext) const { - if (inputContext != inputContext_) { - return; +size_t FilteredForgettableCandidate::candidateIndex() const { + const auto *that = dynamic_cast(this); + if (!that) { + FCITX_ERROR() << "should be a subclass to FilteredCandidateWrord, bug " + "in pinyin engine."; + return 0; } - auto *state = inputContext->propertyFor(&engine_->factory()); + auto *inputContext = that->inputContext(); + auto *engine = that->engine(); + auto index = that->originalIndex(); + + auto *state = inputContext->propertyFor(&engine->factory()); if (!state->strokeCandidateList_ || - state->strokeCandidateList_->toBulk()->totalSize() <= index_) { - FCITX_ERROR() << "Stroke candidate is not consistent. Probably a " - "bug in implementation"; - return; + state->strokeCandidateList_->toBulk()->totalSize() <= index) { + return 0; } // Forward the selection to internal candidate list. - state->strokeCandidateList_->toBulk()->candidateFromAll(index_).select( - inputContext); - engine_->resetStroke(inputContext); + const auto &candidate = + state->strokeCandidateList_->toBulk()->candidateFromAll(index); + + if (const auto *forgettable = + dynamic_cast(&candidate); + forgettable != this) { + return forgettable->candidateIndex(); + } + return 0; } ForgetCandidateWord::ForgetCandidateWord(PinyinEngine *engine, Text text, diff --git a/im/pinyin/pinyincandidate.h b/im/pinyin/pinyincandidate.h index 73ff136..aca7b01 100644 --- a/im/pinyin/pinyincandidate.h +++ b/im/pinyin/pinyincandidate.h @@ -123,16 +123,16 @@ class PinyinPunctuationCandidateWord : public CandidateWord { std::string word_; }; -class StrokeFilterCandidateWord : public CandidateWord, - public InsertableAsCustomPhraseInterface, - public ForgettableCandidateInterface { +class FilteredCandidateWord : public CandidateWord { public: - StrokeFilterCandidateWord(PinyinEngine *engine, InputContext *inputContext, - Text text, int index); + FilteredCandidateWord(PinyinEngine *engine, InputContext *inputContext, + Text text, int index); void select(InputContext *inputContext) const override; - std::string customPhraseString() const override; - size_t candidateIndex() const override; + + int originalIndex() const { return index_; } + PinyinEngine *engine() const { return engine_; } + InputContext *inputContext() const { return inputContext_; } private: PinyinEngine *engine_; @@ -140,6 +140,23 @@ class StrokeFilterCandidateWord : public CandidateWord, int index_; }; +class FilteredInsertableAsCustomPhrase + : public InsertableAsCustomPhraseInterface { +public: + std::string customPhraseString() const override; +}; + +class FilteredForgettableCandidate : public ForgettableCandidateInterface { +public: + size_t candidateIndex() const override; +}; + +template +class StrokeFilterCandidateWord : public FilteredCandidateWord, public Args... { +public: + using FilteredCandidateWord::FilteredCandidateWord; +}; + class ForgetCandidateWord : public CandidateWord { public: ForgetCandidateWord(PinyinEngine *engine, Text text, size_t index); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3c19df1..994a4d1 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -14,7 +14,7 @@ endif() add_executable(testpinyinhelper testpinyinhelper.cpp) target_link_libraries(testpinyinhelper Fcitx5::Core Fcitx5::Module::PinyinHelper) -add_dependencies(testpinyinhelper pinyinhelper pinyinhelper.conf.in-fmt) +add_dependencies(testpinyinhelper pinyin pinyin.conf.in-fmt pinyinhelper pinyinhelper.conf.in-fmt) add_test(NAME testpinyinhelper COMMAND testpinyinhelper) add_executable(testfullwidth testfullwidth.cpp) diff --git a/test/testpinyin.cpp b/test/testpinyin.cpp index ba1ca45..b9da593 100644 --- a/test/testpinyin.cpp +++ b/test/testpinyin.cpp @@ -31,7 +31,7 @@ using namespace fcitx; std::unique_ptr endTestEvent; void testPunctuationPart2(Instance *instance); -int findCandidateOrDie(InputContext *ic, std::string_view word) { +int findCandidate(InputContext *ic, std::string_view word) { auto candList = ic->inputPanel().candidateList(); for (int i = 0; i < candList->toBulk()->totalSize(); i++) { const auto &candidate = candList->toBulk()->candidateFromAll(i); @@ -39,10 +39,15 @@ int findCandidateOrDie(InputContext *ic, std::string_view word) { return i; } } - FCITX_ASSERT(false) << "Failed to find candidate: " << word; return -1; } +int findCandidateOrDie(InputContext *ic, std::string_view word) { + auto index = findCandidate(ic, word); + FCITX_ASSERT(index >= 0) << "Failed to find candidate " << word; + return index; +} + void findAndSelectCandidate(InputContext *ic, std::string_view word) { auto candList = ic->inputPanel().candidateList(); candList->candidate(findCandidateOrDie(ic, word)).select(ic); @@ -252,6 +257,41 @@ void testForget(Instance *instance) { }); } +void testActionInStrokeFilter(Instance *instance) { + instance->eventDispatcher().schedule([instance]() { + auto *testfrontend = instance->addonManager().addon("testfrontend"); + auto uuid = + testfrontend->call("testapp"); + auto *ic = instance->inputContextManager().findByUUID(uuid); + FCITX_ASSERT(ic); + + testfrontend->call(uuid, Key("Control+space"), + false); + // Target ppp for 彡 + testfrontend->call(uuid, Key("p"), false); + testfrontend->call(uuid, Key("p"), false); + testfrontend->call(uuid, Key("p"), false); + auto *candidateList = ic->inputPanel().candidateList().get(); + findCandidateOrDie(ic, "彡"); + testfrontend->call(uuid, Key("`"), false); + testfrontend->call(uuid, Key("p"), false); + testfrontend->call(uuid, Key("p"), false); + testfrontend->call(uuid, Key("p"), false); + candidateList = ic->inputPanel().candidateList().get(); + FCITX_ASSERT(findCandidate(ic, "彡") < 0); + int index = findCandidateOrDie(ic, "䫠"); + const auto &cand = candidateList->candidate(index); + auto *actionable = candidateList->toActionable(); + FCITX_ASSERT(actionable); + FCITX_ASSERT(actionable->hasAction(cand)); + auto actions = actionable->candidateActions(cand); + FCITX_ASSERT(!actions.empty()); + FCITX_ASSERT(actions[0].id() == 0); + actionable->triggerAction(cand, 0); + FCITX_ASSERT(ic->inputPanel().candidateList()); + }); +} + void testPin(Instance *instance) { instance->eventDispatcher().schedule([instance]() { auto *testfrontend = instance->addonManager().addon("testfrontend"); @@ -272,7 +312,8 @@ void testPin(Instance *instance) { FCITX_ASSERT(ic); auto index1 = findCandidateOrDie(ic, "同音"); auto index2 = findCandidateOrDie(ic, "痛饮"); - const auto oldIndex1 = index1, oldIndex2 = index2; + const auto oldIndex1 = index1; + const auto oldIndex2 = index2; FCITX_INFO() << "同音:" << index1 << " " << "痛饮:" << index2; { @@ -411,6 +452,7 @@ int main() { testSelectByChar(&instance); testUppercase(&instance); testForget(&instance); + testActionInStrokeFilter(&instance); testPin(&instance); testPunctuation(&instance); instance.exec();