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

Larkin: Layout independent bindings #56

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

haberdashPI
Copy link
Owner

@haberdashPI haberdashPI commented Jan 13, 2025

Closes #54, addressing the following points:

  1. Replace all uses of keys in Larkin with the layout independent versions
  2. Nice to have: make sure it is easy to ignore all these layout independent versions via foreach. I think this might already be possible by using foreach.key = ['{key: [[A-Za-z0-9]+]}', 'shift+{key: [[A-Za-z0-9]+]}'], but I need to verify that it is working (and fix it if it isn't).
  3. Handle display of layout independent bindings in the Masterkey Visual Documentation: Keep US layout but verify all is well
  4. Verify that the layout independent bindings display properly in the quick pick view — fix them if they aren't working

Remaining TODOs:

  • Cleanup partial layout independence (e.g. replace [r] with [KeyR]).
  • Review layout independence of all keys in Larkin.toml
  • Create layout dependent version of Larking.toml
  • Think through discoverability of layout independence. Right now I'm not super convinced it will be clear to users.
  • Fix false positives for (U.S. Layout) suffix
  • Verify that the layout features are working in the wild (not just in tests).

@haberdashPI haberdashPI marked this pull request as draft January 13, 2025 02:25
@haberdashPI haberdashPI force-pushed the push-sszrsmrxzzwo branch 8 times, most recently from 608db6f to 569e096 Compare January 18, 2025 17:12
@haberdashPI haberdashPI marked this pull request as ready for review January 18, 2025 17:13
@haberdashPI
Copy link
Owner Author

haberdashPI commented Jan 20, 2025

Note for WIP

Debugging using version 1.92 of VSCode, as that is the last version that supports debugging a web extension right now (reported bug is merged, but needs to make into a release—it's not even in the insiders release yet it seems).

foreach of digits is not yet working, guessing this is some issue with escaping the brackets in the regex.

@haberdashPI haberdashPI force-pushed the push-sszrsmrxzzwo branch 2 times, most recently from bcd9f34 to f4840d2 Compare January 22, 2025 04:19
Copy link

codecov bot commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.83%. Comparing base (8e0b701) to head (ee5d849).

Files with missing lines Patch % Lines
src/web/commands/palette.ts 85.71% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   79.44%   79.83%   +0.39%     
==========================================
  Files          24       25       +1     
  Lines        2340     2371      +31     
  Branches      468      473       +5     
==========================================
+ Hits         1859     1893      +34     
+ Misses        292      289       -3     
  Partials      189      189              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@haberdashPI haberdashPI force-pushed the push-sszrsmrxzzwo branch 4 times, most recently from 57183e1 to 4850dc3 Compare January 25, 2025 21:05
@haberdashPI
Copy link
Owner Author

@sanarise: This branch is now in working order, and includes support for all of the bullets listed above. Would you be willing to take this version out for a "test drive"??

  • see if your own keybinding file works well with these change
  • try out the default (newly layout independent) Larking bindings to see if you find any issues working with it for just a little bit.

Happy to help walk you through the steps to get this to compile and install for you.

@sanarise
Copy link

Hi! Yes, apparently I need instructions on how to compile this branch from source and connect it to my vscode. I've cloned the repository and played around with the branch and various commands from the package.json, but I haven't figured out yet how to get the compiled module and how to connect it to my vscode. Seems like its need to build a vsix file/module?
2025-01-27_10-00-37

@haberdashPI
Copy link
Owner Author

First step should be to follow the directions at the end of the README.

Once those steps are complete you can run

mise use vsce
vsce package

And you should have a VSIX package you can install.

Alternatively, you could also run the "Debug: Start Debugging" command within the open VSCode project.

@sanarise
Copy link

  1. I have an error when trying to use vsce in mise
    2025-01-28_12-09-00
  2. I started the project in debug mode, and tried my preset - everything seems to work with LI bindings. But if you replace "[Semicolon]" with ";" in the rule, it works in the English layout, but not in the Russian one. At the same time, in the modline, when the key is pressed, it shows "[;]" and, accordingly, this is not worked out in any way. Is this a bug or a feature?

@sanarise
Copy link

I'm not sure that the idea I'm going to propose now is clear in itself and has no pitfalls. But now there is still one oddity in the behavior.
For letter keys, you can bind both "a" and "KeyA" and everything will work in all layouts. It's been like this since the beginning. And it masks the presence of this problem, which is also bad. For non-letter ones, only "[Semicolon]" works smoothly, but ";" also causes a problem in national layouts.

Maybe you should think about hiding this problem from the user altogether? That is, at the toml preset level, everything remains as it is (without LI-bindings), and inside the MasterKey, always translate to LI-bindings. Can there be any arguments against this? That is, we may not be able to map to LI-bindings under some conditions, but it's still hard for me to imagine when it's needed...

@haberdashPI
Copy link
Owner Author

haberdashPI commented Jan 28, 2025

have an error when trying to use vsce in mise

Ah, I forgot. It is mise use npm:vsce. Whoopsie.

Maybe you should think about hiding this problem from the user altogether?

I'm not convinced. It is a little unintuitive that you might have a binding for one letter or symbol, but you hit the key for some other symbol on your keyboard to use that binding: it might be something developers with international layouts want but a user new to all of this could find it very confusing (and maybe think it's a bug).

For non-letter ones, only "[Semicolon]" works smoothly, but ";" also causes a problem in national layouts.

What do you mean only [Semicolon] works smoothly?

it shows "[;]" and, accordingly, this is not worked out in any way

I'm a little lost about what you mean here. The intent is that [;] means you are pressing what would be, in a US Layout, the ; key. There isn't any easy way to know what key it is for any given layout (I don't think extensions have access to the current layout).

What is the behavior you were expecting?

Maybe you could spell out the following for a few key examples:

  • list what key you are hitting on your keyboard
  • what key you expect it to appear to the extension while in normal mode
  • what key the extension actually registers

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.

The problem with self-input non-alphabetic keys in non-latin keyboard layouts
2 participants