Skip to content
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

feat(translator): allow a translator to take multiple tags #926

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

ksqsf
Copy link
Member

@ksqsf ksqsf commented Aug 14, 2024

Pull request

Feature

Allow script_translator and table_translator to take multiple tags.

translator:
  tags: [abc, tag1, tag2, tag3]

To keep compatibility:

  1. if there is only tag: tags is set to that tag
  2. if there is only tags: tags is set to that list
  3. if there are both tag and tags: tag is the first element in the list, and tags is understood similar to extra_tags

Unit test

  • Done

Manual test

  • Done

Code Review

  1. Unit and manual test pass
  2. GitHub Action CI pass
  3. At least one contributor reviews and votes
  4. Can be merged clean without conflicts
  5. PR will be merged by rebase upstream base

Additional Info

@ksqsf ksqsf changed the title feat(translator_commons): allow a translator to take multiple tags feat(translator): allow a translator to take multiple tags Aug 14, 2024
@@ -146,8 +146,10 @@ class TranslatorOptions {
bool IsUserDictDisabledFor(const string& input) const;

const string& delimiters() const { return delimiters_; }
const string& tag() const { return tag_; }
void set_tag(const string& tag) { tag_ = tag; }
vector<string> tags() const { return tags_; }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not const vector<string>& due to a compilation eror in librime-lua which I don't know how to solve.

@ksqsf ksqsf requested a review from a team August 15, 2024 08:56
@@ -132,6 +131,20 @@ TranslatorOptions::TranslatorOptions(const Ticket& ticket) {
config->GetList(ticket.name_space + "/comment_format"));
user_dict_disabling_patterns_.Load(
config->GetList(ticket.name_space + "/disable_user_dict_for_patterns"));
string tag;
if (config->GetString(ticket.name_space + "/tag", &tag)) {
// replace the first tag, and undersand /tags as extra tags
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is undersand a typo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed.

also fixed a case where the invariant could be broken.

@ksqsf ksqsf merged commit d47a812 into rime:master Sep 21, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants