-
Notifications
You must be signed in to change notification settings - Fork 630
feat(candidate): allow arbitrary per-candidate data #1092
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a per-candidate data store so plugins can attach arbitrary data to Candidate instances, enabling action-style candidates and other plugin-driven behaviors.
- Introduces set_data/get_data/get_data_keys public APIs on Candidate
- Adds an internal map to hold arbitrary data keyed by strings
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
// per-candidate data | ||
void set_data(const string& data_tag, an<void> datum = nullptr) { | ||
data_[data_tag] = datum; | ||
} | ||
an<void> get_data(const string& data_tag) { | ||
auto it = data_.find(data_tag); | ||
return it != data_.end() ? it->second : nullptr; | ||
} | ||
vector<string> get_data_keys() { | ||
vector<string> keys; | ||
for (const auto& pair : data_) { | ||
keys.push_back(pair.first); | ||
} | ||
return keys; | ||
} |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new public APIs need clear documentation: ownership semantics of the stored pointers (who creates/destroys), expected type-erasure/casting pattern, whether nullptr is a supported value, thread-safety guarantees, and that get_data_keys returns keys in unspecified order. Please add brief doc comments for each method and the data_ member describing these behaviors.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation is not a rime tradition ;-)
void set_quality(double quality) { quality_ = quality; } | ||
|
||
// per-candidate data | ||
void set_data(const string& data_tag, an<void> datum = nullptr) { |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using void-erased pointers for arbitrary data is unsafe and forces consumers to manage casting/lifetimes. If feasible for the codebase standard, consider std::any (or an existing project-wide any/variant) for type-safe storage while still allowing arbitrary data.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any
is safer but is not naturally nullable, and optional<any>
seems to have too many overheads.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
@lotem 来点反馈? 这个功能本身肯定是有必要的。比如说可以把 spelling hints 放到 data 里,这样就不需要占用 comment。 具体设计上可以再探讨。一个可能的简化法是改成 map<string, string>。 |
好的。 可否提供带模板参数的 get/set,因为用家总要做类型转换。 |
允許給
Candidate
增加任意用字符串索引的數據。(此項變更僅面向插件,用戶無法直接使用。)爲 Candidate 增加三個新的公開接口:
插件可調用該接口,在產生 Candidate 時,附加任意數據。其中,datum 可設爲 null,則作用類似於 tags,但粒度是 Candidate。
一個可能的用例是:與 librime-lua 整合後,可以讓用戶編寫任意「Action Candidate」,這些 Candidate 不用來上屏文字,而是觸發任意邏輯。如:
F
,得到候選「切換爲繁體」,選擇後不上屏任何內容,而是設置 simplification 爲 falseJ
,得到候選「切換爲簡體」,選擇後不上屏任何內容,而是設置 simplification 爲 true接口本身還可以進一步討論。不過因爲這個接口只用於插件(考慮到插件已經和 librime 本體強綁定),日後要修改或許也不會有太大困難。