Keycodes refactor #171

wismill · 2024-04-07T20:55:36Z

Simplify code and allow direct mapping of keys.

The new keys are only supported by XKB atm.

- Moved all the info about scan codes in the YAML file - Added a dedicated class `Key` that gather all the scan codes - Simplify the code

Using ASCII art to declare the layout is limited to the keys declared in the geometries. Added an extra section `mapping` to the TOML layout configuration file, that enable mappings key explicitly with a dictionary.

wismill · 2024-04-07T21:50:33Z

@fabi1cazenave I left as draft as it:

still misses tests
need to add warnings if key is not supported

Use full scan codes, in preparation for all key remapping.

wismill · 2024-04-08T07:09:18Z

Added tests for XKB.
Added Windows scan codes. Require writing tests, but above all testing it @Geobert.

wismill · 2024-04-08T09:45:01Z

Added test for Windows and improve VK handling, especially to avoid same VK with multiple mappings.

fabi1cazenave

You probably didn’t want any review at this stage but I had to give a look. :-) Impressive work !

I’m not sure the hand name is very self-explanatory. And I’d like to see a TOML file with your custom mapping proposal. ^_^

kalamine/data/qwerty_vk.yaml

fabi1cazenave · 2024-04-09T19:38:34Z

kalamine/data/scan_codes.yaml

+  web: "Digit3"
+  windows: "T04"
+  macos: "20"
+  hand: null


Nitpick, maybe the following could be easier to read :

digits: { web: Digit1, xkb: ae01, windows: T02, macos: "18", hand: null } { web: Digit3, xkb: ae02, windows: T03, macos: "19", hand: null } { web: Digit3, xkb: ae03, windows: T04, macos: "20", hand: null }

FTR & TBH I disliked this change… until I saw how it improves the code readability.

Good idea. But wouldn’t a CSV format be even more adapted then?

kalamine/generators/klc.py

kalamine/generators/web.py

kalamine/generators/xkb.py

fabi1cazenave · 2024-04-09T20:03:46Z

kalamine/key.py

+        return bool(self.category & KeyCategory.AlphaNum)
+
+
+KEYS = Key.load_data(load_data("scan_codes"))


This line triggers my PTSD. :-)

I like data classes but I’m not a fan of creating void objects first, then fill them with data. This looks like an anti-pattern to me.

Couldn’t we use the constructor instead for that , and drop all parse / load_data methods in these data classes ? Otherwise, factory functions parsing data and returning such objects would make sense.

As discussed in the Discord server, we are not creating void objects. It is merely putting the parsing functions under a proper namespace.

I think you mean the initializer __init__, not the constructor __new__.

It is not a good idea to modify the initializer nor the default constructor, because they are created automatically with @dataclass.

But in fact Key.parse is just another constructor, as you cannot overload functions properly in Python.

But the Key.load_data method has a poor name. Fixed.

fabi1cazenave · 2024-04-09T20:07:24Z

kalamine/utils.py

@@ -47,15 +47,33 @@ class Layer(IntEnum):
    ALTGR = 4
    ALTGR_SHIFT = 5

+    @classmethod
+    def parse(cls, raw: str) -> Optional["Layer"]:
+        rawʹ = raw.casefold()


this is probably a dumb question but is this trailing single quote sign a special Python stuff ?

That is not Python specific: it a prime, and I find it very practical for closely related variables. Common practice from Haskell I guess.

fabi1cazenave · 2024-04-09T20:10:13Z

tests/test_serializer_klc.py

+        "esc": {"base": "\x1b", "shift": "(ae11)"},
+        # NOTE: clone key
+        "henk": "(lsgt)",
+    }


I assume this is a work in progress. Otherwise the comments could be more explicit, I’m not following you here. 🙇

Updated. Is it clearer?

fabi1cazenave · 2024-04-09T20:15:19Z

kalamine/utils.py

@@ -47,15 +47,33 @@ class Layer(IntEnum):
    ALTGR = 4
    ALTGR_SHIFT = 5

+    @classmethod
+    def parse(cls, raw: str) -> Optional["Layer"]:


As mentioned in another comment, I’m not a fan of having a parse method to be called after the object has been instantiated. To me it should either be a constructor, or we should have a factory that returns such an object.

wismill

Updated a few things

kalamine/data/qwerty_vk.yaml

wismill · 2024-04-10T05:14:55Z

kalamine/key.py

+        return bool(self.category & KeyCategory.AlphaNum)
+
+
+KEYS = Key.load_data(load_data("scan_codes"))


As discussed in the Discord server, we are not creating void objects. It is merely putting the parsing functions under a proper namespace.

I think you mean the initializer __init__, not the constructor __new__.

It is not a good idea to modify the initializer nor the default constructor, because they are created automatically with @dataclass.

But in fact Key.parse is just another constructor, as you cannot overload functions properly in Python.

wismill · 2024-04-10T05:16:40Z

kalamine/key.py

+        return bool(self.category & KeyCategory.AlphaNum)
+
+
+KEYS = Key.load_data(load_data("scan_codes"))


But the Key.load_data method has a poor name. Fixed.

wismill · 2024-04-10T05:53:03Z

kalamine/data/scan_codes.yaml

+  web: "Digit3"
+  windows: "T04"
+  macos: "20"
+  hand: null


Good idea. But wouldn’t a CSV format be even more adapted then?

wismill · 2024-07-05T11:23:16Z

Not going to work on kalamine anymore.

wismill force-pushed the keycodes/refactor branch 6 times, most recently from cf73dfa to f418454 Compare April 7, 2024 21:37

wismill added 4 commits April 7, 2024 23:39

Refactor scan codes

22bb7a1

- Moved all the info about scan codes in the YAML file - Added a dedicated class `Key` that gather all the scan codes - Simplify the code

feat: Add extra key mapping section

3b7a506

Using ASCII art to declare the layout is limited to the keys declared in the geometries. Added an extra section `mapping` to the TOML layout configuration file, that enable mappings key explicitly with a dictionary.

Fix Enum eq comparison

cfa19db

Minor fix

d8be6a9

wismill force-pushed the keycodes/refactor branch from f418454 to d8be6a9 Compare April 7, 2024 21:40

wismill added 3 commits April 8, 2024 08:58

Add Numpad keys & more web key codes

91cf7f0

Add Windows scan codes

4fa8b2f

Use full scan codes, in preparation for all key remapping.

test(XKB): Add extra mapping test

c866103

wismill force-pushed the keycodes/refactor branch from ac701dc to c866103 Compare April 8, 2024 07:06

wismill added 2 commits April 8, 2024 11:41

Improve Windows VK handling

482d30e

test(klc): Add extra mapping test

cc53a66

fabi1cazenave added the enhancement New feature or request label Apr 9, 2024

fabi1cazenave reviewed Apr 9, 2024

View reviewed changes

Review fixes

9c76a5e

wismill commented Apr 10, 2024

View reviewed changes

wismill closed this Jul 5, 2024

fabi1cazenave reopened this Aug 25, 2024

Merge branch 'main' into keycodes/refactor

fa6a1b2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keycodes refactor #171

Keycodes refactor #171

wismill commented Apr 7, 2024 •

edited

Loading

wismill commented Apr 7, 2024

wismill commented Apr 8, 2024

wismill commented Apr 8, 2024

fabi1cazenave left a comment

fabi1cazenave Apr 9, 2024

wismill Apr 10, 2024

fabi1cazenave Apr 9, 2024

wismill Apr 10, 2024

wismill Apr 10, 2024

fabi1cazenave Apr 9, 2024

wismill Apr 9, 2024

fabi1cazenave Apr 9, 2024

wismill Apr 10, 2024 •

edited

Loading

fabi1cazenave Apr 9, 2024

wismill left a comment

wismill Apr 10, 2024

wismill Apr 10, 2024

wismill Apr 10, 2024

wismill commented Jul 5, 2024

		return bool(self.category & KeyCategory.AlphaNum)


		KEYS = Key.load_data(load_data("scan_codes"))

Keycodes refactor #171

Are you sure you want to change the base?

Keycodes refactor #171

Conversation

wismill commented Apr 7, 2024 • edited Loading

wismill commented Apr 7, 2024

wismill commented Apr 8, 2024

wismill commented Apr 8, 2024

fabi1cazenave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wismill Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wismill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wismill commented Jul 5, 2024

wismill commented Apr 7, 2024 •

edited

Loading

wismill Apr 10, 2024 •

edited

Loading