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.

Sign up for GitHub

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: support import/display/export of extra host keyboard layouts / languages #1374

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

feat: support import/display/export of extra host keyboard layouts / languages #1374

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f526ee0
feat: support import/render/export of other keyboard layouts
tenstad Nov 11, 2024
5299850
fix: dead keys
tenstad Nov 11, 2024
ac2f809
fix: simply add custom language codes to lookup
tenstad Nov 11, 2024
5757e24
feat: support all languages - not just selected
tenstad Nov 12, 2024
db616fc
feat: display language codes
tenstad Nov 13, 2024
e8a778f
fix: only show language when showing keymap
tenstad Nov 14, 2024
67af324
fix: only show language when showing keymap
tenstad Nov 14, 2024
71654df
fix: improved keycode extraction from title
tenstad Nov 14, 2024
ce363a2
fix: only display language-specific codes for chosen host language
tenstad Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/store/keycodes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,16 @@ function generateKeycodes(osKeyboardLayout, isSteno = false) {
...(isSteno ? steno : [])
];
const { keycodeLUT } = keymapExtras[getOSKeyboardLayout()];
return keycodes.map((keycodeObject) =>
toLocaleKeycode(keycodeLUT, keycodeObject)
);
return [
...keycodes.map((keycodeObject) =>
toLocaleKeycode(keycodeLUT, keycodeObject)
),
...Object.entries(keycodeLUT).map(([code, { name, title }]) => ({
code: title?.split(' ')[0],
name,
title: code
}))
];
Copy link
Author

Choose a reason for hiding this comment

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

We could in theory just add all languages like this, so that one can upload NO_, FR_ and everything else regardless of what language is configured in Settings. Would be easier to use, and I don't see any harm except maybe longer searches for match.

Copy link
Author

@tenstad tenstad Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Implemented in 5757e24 - let me know what you think.

Works for FR_CCED and IL_MUL at the same time as NO_ (not that you would have a keymap like that):
image image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what the value of that is, given that the interpretation is based on the OS setting. Don't you think this would be more confusing than helpful?

Copy link
Author

Choose a reason for hiding this comment

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

Can agree that it's a bit confusing when having multiple languages at the same time, but for that to happen, the user would have to configure it on their own before importing. It could also be useful for some to have multiple languages in different layers if they jump between computers with different host languages.

The nice thing here is that users do not need to remember nor know that they have to to configure host language before importing their NO_ or FR_ keyboard. As they use language-specific codes, they would probably never select the correct host language - because that just gives them the corresponding/mapped KC_ codes, not NO_ or FR? Thus I think it's nice that the tool actually displays the NO and _FR codes correctly with the default host language setting.

I think that is more valuable than potentially confusing those with multiple language codes in the same layout.

Copy link
Author

@tenstad tenstad Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We could put it behind a feature toggle in the settings. support extra language keycodes or something. However, I then against fear some will think the tool simply does not support it, as they try without going into settings. So would prefer not to.

Copy link
Contributor

@precondition precondition Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this will reinforce even more the incorrect preconception that keycodes like FR_CCED will invariably produce the "ç" keysym no matter the OS settings because that is what it says on the tin can after all.

It would be nice to properly support JSON files with mixed aliases but the displayed layout should obey the host OS keyboard layout setting or else the setting will lose meaning. So, I'd vote against the display of mixed aliases.

For a keymap.json like:

["FR_CCED", "IL_MUL", "LSFT_T(NO_T)", "NO_G"]

I'd expect these legends to look like this under "English (USA)":

(
9		 RAlt
—
*
8
—		 LSft
—
T
—		 G

Although one problem with this approach is that there may exist multiple keymap_extras variants for the same language (e.g. keymap_french.h and keymap_french_afnor.h) and then it becomes ambiguous whether FR_CCED should translate to KC_9 or to ALGR(KC_C).

The host_language field in the JSON could potentially help to solve the ambiguity but as far as I can tell that only accepts a single host language and if we're dealing with JSON files mixing aliases from multiple keymap_extra aliases, who knows which host_language will be picked (it might be "norwegian" instead of either "french" or "french_afnor" in which case the translation becomes ambiguous)

Copy link
Author

@tenstad tenstad Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is how it's looking in the Zsa configurator btw. First have to enable the language, and can then use it. By having to enable it before the relevant keys are possible to select, the information is at least somewhat conveyed to the user.

Screenshot_20241113_192022_Chrome.jpg

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still in the middle of moving to pinia so I'm not sure I want this disruption right now for what is a pretty confusing and niche use case.

Copy link
Author

Choose a reason for hiding this comment

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

Not completely comparable as the qmk configurator does not support setting the language-specific keycodes in the GUI. Here, it only applies to keymaps that already contain language-specific key codes on import.

I think the main thing I would like to accomplish here is 1. to not change the keycodes when importing and exporting a keymap if no modifications are made, and 2. ideally somewhat indicate what's actually in the JSON file for that key.

Other than that, I'm very open to making modifications.

Copy link
Contributor

@precondition precondition Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I can already imagine the case of the German programmer who hates having to use AltGr to access [] and {} and prefers the [ { and ] } keysyms found on US layouts but still wants access to the ß, ü, ä and ö keys she needs to type German. Mixing the US [ { keys with the German ß key sounds like the perfect solution except it won't work as she expects.

In fact, I remember multiple people on Discord tripping up over pretty much the same thing because they wanted the US shift pairs and thus used the default KC_ keycodes everywhere in their keymap and then imported a keymap_extra just "to get access" to a special key, unavailable in vanilla US, and were confused why the special key they added to their keymap was not producing the expected result.

EDIT: That said, it is not as if those "hardcoded" localized aliases are available in the GUI so maybe we can make the assumption that if someone includes those in a JSON file, (s)he must have inserted them manually on purpose and may know better what are the implications of doing that.

}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/store/modules/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ function parseKeycode(keycodesStore, keycode, stats) {
}
let internal = splitcode[1];
internal = internal.split(')')[0];
metadata = keycodesStore.lookupKeycode(internal);

// check for an OSM keycode
if (maincode === 'OSM') {
Expand All @@ -189,13 +190,12 @@ function parseKeycode(keycodesStore, keycode, stats) {
}

//Check whether it is a layer switching code, mod-tap, or combo keycode
if (internal.includes('KC')) {
if (internal.includes('KC') || metadata?.title?.includes('KC')) {
// Layer Tap keycode
if (maincode === 'LT') {
return newLayerContainerKey(keycodesStore, maincode, internal);
}
internal = longFormKeycodes[internal] || internal;
metadata = keycodesStore.lookupKeycode(internal);
if (metadata === undefined) {
stats.any += 1;
return newAnyKey(keycodesStore, keycode);
Expand Down