Skip to content

Commit

Permalink
Fix stroke filter candidate action (#207)
Browse files Browse the repository at this point in the history
Filtering candidates wrongly assume candidate can only be pinyin
candidate type. Use dynamic_cast to insert candidate that can be casted.
  • Loading branch information
wengxt authored Dec 22, 2024
1 parent ed1a886 commit cbcae46
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 40 deletions.
14 changes: 12 additions & 2 deletions im/pinyin/pinyin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1464,8 +1464,18 @@ void PinyinEngine::updateStroke(InputContext *inputContext) {
}
}
if (strokeMatched) {
candidateList->append<StrokeFilterCandidateWord>(
this, inputContext, candidate.text(), i);
// PinyinCandidateWord is the only forgettable.
if (dynamic_cast<const PinyinCandidateWord *>(&candidate)) {
candidateList->append<StrokeFilterCandidateWord<
FilteredForgettableCandidate,
FilteredInsertableAsCustomPhrase>>(this, inputContext,
candidate.text(), i);
} else if (dynamic_cast<const InsertableAsCustomPhraseInterface
*>(&candidate)) {
candidateList->append<StrokeFilterCandidateWord<
FilteredInsertableAsCustomPhrase>>(this, inputContext,
candidate.text(), i);
}
}
}
}
Expand Down
83 changes: 56 additions & 27 deletions im/pinyin/pinyincandidate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<const PinyinCandidateWord &>(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<const FilteredCandidateWord *>(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<const PinyinCandidateWord &>(candidate).candidateIndex();
state->strokeCandidateList_->toBulk()->candidateFromAll(index);
if (const auto *insertable =
dynamic_cast<const InsertableAsCustomPhraseInterface *>(&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<const FilteredCandidateWord *>(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<const ForgettableCandidateInterface *>(&candidate);
forgettable != this) {
return forgettable->candidateIndex();
}
return 0;
}

ForgetCandidateWord::ForgetCandidateWord(PinyinEngine *engine, Text text,
Expand Down
31 changes: 24 additions & 7 deletions im/pinyin/pinyincandidate.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,23 +123,40 @@ 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_;
InputContext *inputContext_;
int index_;
};

class FilteredInsertableAsCustomPhrase
: public InsertableAsCustomPhraseInterface {
public:
std::string customPhraseString() const override;
};

class FilteredForgettableCandidate : public ForgettableCandidateInterface {
public:
size_t candidateIndex() const override;
};

template <typename... Args>
class StrokeFilterCandidateWord : public FilteredCandidateWord, public Args... {
public:
using FilteredCandidateWord::FilteredCandidateWord;
};

class ForgetCandidateWord : public CandidateWord {
public:
ForgetCandidateWord(PinyinEngine *engine, Text text, size_t index);
Expand Down
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 45 additions & 3 deletions test/testpinyin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,23 @@ using namespace fcitx;
std::unique_ptr<EventSourceTime> 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);
if (candidate.text().toString() == 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);
Expand Down Expand Up @@ -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<ITestFrontend::createInputContext>("testapp");
auto *ic = instance->inputContextManager().findByUUID(uuid);
FCITX_ASSERT(ic);

testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("Control+space"),
false);
// Target ppp for 彡
testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("p"), false);
testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("p"), false);
testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("p"), false);
auto *candidateList = ic->inputPanel().candidateList().get();
findCandidateOrDie(ic, "");
testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("`"), false);
testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("p"), false);
testfrontend->call<ITestFrontend::keyEvent>(uuid, Key("p"), false);
testfrontend->call<ITestFrontend::keyEvent>(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");
Expand All @@ -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;
{
Expand Down Expand Up @@ -411,6 +452,7 @@ int main() {
testSelectByChar(&instance);
testUppercase(&instance);
testForget(&instance);
testActionInStrokeFilter(&instance);
testPin(&instance);
testPunctuation(&instance);
instance.exec();
Expand Down

0 comments on commit cbcae46

Please sign in to comment.