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

Incorrect Keyboard Layout Titles #523

Closed
sslater11 opened this issue Oct 21, 2023 · 8 comments
Closed

Incorrect Keyboard Layout Titles #523

sslater11 opened this issue Oct 21, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@sslater11
Copy link
Contributor

sslater11 commented Oct 21, 2023

@KraXen72 could you please help with this 😅
With the current version there are some naming issues I've found.

1.

I believe PTThumbKey should have the title = "PT Thumb-Key português", but it says Type-Split.

PTThumbKey.kt:     title = "PT Type-Split português",
PTTypeSplit.kt:    title = "PT Type-Split português",

2.

Any idea which layout FRTypeSplit.kt is? Is it french, español, or a mix of both?

FRTypeSplit.kt:    title = "ES Type-Split français",
ESTypeSplit.kt:    title = "ES Type-Split español",

3.

I might as well ask here. What is "EN NO Type-Split"? should its title include 'english' and the other language it is for? I'm reading "NO" as an english word and seeing "no type-split" lol.

Images

thumbkey_layout_name1 thumbkey_layout_name2

@sslater11 sslater11 added the bug Something isn't working label Oct 21, 2023
@KraXen72
Copy link
Contributor

  1. i believe you're right
  2. i can't really check from thumb-key since their titles are the same, but i think the fr one should be fr and just has an incorrect title?
  3. i think NO stands for norwegian, atleast that's what it is in the other kebyboards

also, if you're fixing this some layouts have the k in Thumb-Key capitalized and some don't - i'd suggest changing the outliers to the more prominent naming style, so sorting is consistent.

at this point, there are too many layouts for the way they're manually imported & regestered now, i feel like there should be some better code structuring to prevent/minmize silly issues like some layouts being duplicate and some not appearing at all. Or maybe some CI check which checks 1) with regex that all file names are correct depending on some format 2) all keyboard titles (user facing) follow /^[A-Z]{1, 4} (Thumb-Key|MessageEase|Type-split) \w+ (?:[()A-Za-z-]) (untested regex) or something similar 3) all keyboards are imported and registered (with their function) exactly once. 4) all keyboards have a unique id

what do you think about these proposed checks, @dessalines ?

@dessalines
Copy link
Owner

dessalines commented Oct 22, 2023

I'd be good with regex checks in the CI.

Feel free to do PRs to fix those individual naming issues too.

I think tho because having ES Thumb-Key Espanol is redundant, that I'll remove all the codes, and use the full language names at the very beginning.

@sslater11
Copy link
Contributor Author

I think it would be best to make a layout list with sub-menus. Tap "English" and it'll show you all the English layouts. Tags would be handy too, so we could list the main language layouts first, then have a title and divider saying "Français" to have the french English layouts. With tags we could have the English/Esperanto layout display in both the English and Epseranto sub-menus.

I think tags should be 2 letters like EN/FR/ES, to save us from everyone labeling them "French/Français/Francais".

We do have a lot of different layouts for a single language, and just looking at the English ones, I don't know what one to choose, let alone a newcomer. There's more than any other keyboard I've seen, which is pretty cool, but also why I've not uploaded my own layout. I just replace one of the other layouts with my own file lol.

dessalines added a commit that referenced this issue Oct 22, 2023
@dessalines
Copy link
Owner

I pushed up a renaming, take a look.

The keyboard titles IMO shouldn't be the 2-letter codes, because people often don't know, or have to know them.

@KraXen72
Copy link
Contributor

i've written and tested some regex (with named capture groups, feel free to remove them if unneeded) to implement in ci checks (idk how to do that though, so just take the regex)

^(?<langcode>(?:[A-Z]{2}\/?)+) (?<type>(?:[A-Z][a-z0-9]+-?)+?) (?<lang>(?:\p{L}+-?)+?)? ?(?<extra>\S.+)*$ - check if the keyboard name falls in the correct format. you can check it in regex101 (copy the regex first)
image

Then to test for other things, you probably need to write some actual code.

  • check that all keyboards files are imported
    • count lines starting with import === count of .kt files in keyboards dir
  • check that they are all imported exacly once
    • for each line that starts with import , take that line, slice off import , split by ., last = keyboard package name
    • (likely) python equivalent if helpful: line[7:].split(".")[-1]
    • for each <keyboard package name>) should occur exactly once in the file
  • check that all keyboards have a unique id
    • get lines between the one starting enum class KeyboardLayout and last line
    • ^(?:\s*\w+\()(?<id>\d+) extract ids from the lines with this regex into one array
      image
    • there should be no duplicates (for example, convert to set and compare lengths. if there were duplicates, set will be shorter)

@KraXen72
Copy link
Contributor

KraXen72 commented Oct 22, 2023

if you don't want 2 letter codes just yeet the first capture group and space from the first regex

@storvik
Copy link
Contributor

storvik commented Oct 22, 2023

I'm probably the one introducing these bugs, sorry about that! PR #484. Turns out it's hard to get everything right when refactoring every layout file.

Regarding the EN NO, I think it means English Type-Split, with Norwegian alphabet (includes æøå). It should have been named "EN Type-Split multi NO". Personally I would have called it "NO Type-Split".

I agree with the idea of having tags. Maybe it's possible to add tags or languages to KeyboardDefinition class? It could be a list of languages. Also the "choose keyboard" part of settings should be move to it's own screen. It's hard to find the correct layout, a dedicated screen with filtering would be easier.

Maybe

data class KeyboardDefinition(
    val title: String,
    val langs: Array<String>,
    val type: KeyboardType,   // enum - thumb key, type split, etc
    val modes: KeyboardDefinitionModes,
    val settings: KeyboardDefinitionSettings = KeyboardDefinitionSettings(),
)

One last idea of mine is to not use these indices. If we were to use some string instead I think it will be more future proof and we aren't relying on keyboard maintainers increasing the index by 1 for each layout added. This is a breaking change.

@dessalines
Copy link
Owner

I'd recommend not relying on strings to define keyboards, since as this issue shows, their names can and do undergo frequent name changes. Indexes are future proof for that, in the same way that DB ids should always be a serial / int.

I'm not opposed to tags, but don't do any work on them unless our alorma multi-select picker, works with them in a searchable way. Otherwise its pointless work.

@KraXen72 you could do a PR for that adding it to woodpecker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants