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(switches): abbreviate state labels #615

Merged
merged 2 commits into from Feb 12, 2023
Merged

feat(switches): abbreviate state labels #615

merged 2 commits into from Feb 12, 2023

Conversation

lotem
Copy link
Member

@lotem lotem commented Feb 11, 2023

Pull request

Issue tracker

Fixes will automatically close the related issue

Fixes #

Feature

YAML config: a switch can have optional string array abbrev to define
short state labels which do not have to take the first character of the
full state labels.

(switch_translator): folded options now make use of the short labels
defined in the switches configuration.

(rime_api): add function get_state_label_abbreviated, which returns
RimeStringSlice type since the short label can be the first character
of the full label string.

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

YAML config: a switch can have optional string array `abbrev` to define
short state labels which do not have to take the first character of the
full state labels.

(switch_translator): folded options now make use of the short labels
defined in the `switches` configuration.

(rime_api): add function `get_state_label_abbreviated`, which returns
`RimeStringSlice` type since the short label can be the first character
of the full label string.
@LEOYoon-Tsaw
Copy link
Member

The API querying for short label should better return nothing if it's not defined, instead of returning the first char as default.

Because in front end I need a way to tell if user has defined a short label, but if the API return a default value regardlessly, there's no way to tell.

And front end might be better in handling extraction of the first character

@lotem
Copy link
Member Author

lotem commented Feb 12, 2023

What's the exact use case where the front end needs to know whether a short label is defined?
In the current solution the defined abbrev is a replacement for the calculated short label by taking the first Unicode character. I made the API query behavior consistent with the other usage in folded switch options shown in the switcher UI.
Compare

API get state label abbreviated: false - labels of unfolded switches;

API get state label abbreviated: true - labels of folded switches.

Align the two usages so that there are not too many cases to configure. If the first Unicode is not appropriate for short label, define abbrev in the YAML instead; the author can do that even more correctly than the front end.

@LEOYoon-Tsaw
Copy link
Member

LEOYoon-Tsaw commented Feb 12, 2023

I'm imagining 3 modes:

  1. Always long label,
  2. Always short label, if not defined, fall back to first char of long label,
  3. Short label preferred, if not defined, use long label. (this is most likely the best as default behavior)

The 3rd option is suitable for cases where the first char of long label doesn't make sense (for example: 傳 for TC). And people upgrading from 0.16.x won't notice any change.

@lotem
Copy link
Member Author

lotem commented Feb 12, 2023

Formerly we used state labels like 漢字/汉字 to mimic "short" labels instead of being more descriptive. This format is more wide-spread than the 0.16 version.
I cannot imagine a case where the old schéma could work better without the taking the first-Unicode as short label. They will display either all long labels or all short labels, unless the front end hardcoded the logic. To reimplement the 0.15 state change prompt, the user has to at least add abbrev: [中, A], then they could also define abbrev: [ 漢字, 汉字 ] or whatever they prefer to display. This should cover the multi-codepoint emoji case.
If without updating the schéma, they can also opt out the short label option to get the exact same prompts as in 0.16.
The missing part of this feature is letting user configure whether they want long labels or short labels be displayed by the front end.

@lotem
Copy link
Member Author

lotem commented Feb 12, 2023

I'm imagining 3 modes:

  1. Always long label,
  2. Always short label, if not defined, fall back to first char of long label,
  3. Short label preferred, if not defined, use long label. (this is most likely the best as default behavior)

The 3rd option is suitable for cases where the first char of long label doesn't make sense (for example: 傳 for TC). And people upgrading from 0.16.x won't notice any change.

Mode 3 means it's up to the schéma author to define which options are displayed short or long. They can implement their preferences using either mode 1 or mode 2. If long label is the dominant case, then define states: [中, A] would be a perfect solution and some users are already doing this in squirrel 0.16.

@LEOYoon-Tsaw
Copy link
Member

LEOYoon-Tsaw commented Feb 12, 2023

Well, that's fine
But still, frontend can ensure more accurate first display character extraction (instead of first unicode), and no need to define a string slice struct

@lotem lotem merged commit d0d227c into master Feb 12, 2023
@lotem lotem deleted the state-label branch February 12, 2023 15:33
@LEOYoon-Tsaw
Copy link
Member

Address #608

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.

None yet

2 participants