feat(candidate): allow arbitrary per-candidate data#1092
Conversation
There was a problem hiding this comment.
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; | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
[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.
There was a problem hiding this comment.
any is safer but is not naturally nullable, and optional<any> seems to have too many overheads.
There was a problem hiding this comment.
Null pointers sounds like absence of record / deletion, as you compared it to optional<any>.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@lotem 来点反馈？ 这个功能本身肯定是有必要的。比如说可以把 spelling hints 放到 data 里，这样就不需要占用 comment。 具体设计上可以再探讨。一个可能的简化法是改成 map<string, string>。 |
|
好的。 可否提供带模板参数的 get/set，因为用家总要做类型转换。 |
| vector<string> keys; | ||
| keys.reserve(data_.size()); | ||
| for (const auto& pair : data_) { | ||
| keys.push_back(pair.first); |
There was a problem hiding this comment.
Check non-null?
Null pointers should be treated as absent.
| void set_quality(double quality) { quality_ = quality; } | ||
|
|
||
| // per-candidate data | ||
| void set_data(const string& data_tag, an<void> datum = nullptr) { |
There was a problem hiding this comment.
Null pointers sounds like absence of record / deletion, as you compared it to optional<any>.
| } | ||
| an<void> get_data(const string& data_tag) const { | ||
| auto it = data_.find(data_tag); | ||
| return it != data_.end() ? it->second : nullptr; |
There was a problem hiding this comment.
Another proof: null pointer and absence are equal.
|
之后改一下。目前有个想法是，大部分用例应该都是 string 类型（candidate 级的 tag），所以应该用类似 |
允許給
Candidate增加任意用字符串索引的數據。（此項變更僅面向插件，用戶無法直接使用。）爲 Candidate 增加三個新的公開接口：
插件可調用該接口，在產生 Candidate 時，附加任意數據。其中，datum 可設爲 null，則作用類似於 tags，但粒度是 Candidate。
一個可能的用例是：與 librime-lua 整合後，可以讓用戶編寫任意「Action Candidate」，這些 Candidate 不用來上屏文字，而是觸發任意邏輯。如：
F，得到候選「切換爲繁體」，選擇後不上屏任何內容，而是設置 simplification 爲 falseJ，得到候選「切換爲簡體」，選擇後不上屏任何內容，而是設置 simplification 爲 true接口本身還可以進一步討論。不過因爲這個接口只用於插件（考慮到插件已經和 librime 本體強綁定），日後要修改或許也不會有太大困難。