-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix Keysym names for named Keysyms and test them agains xkbcommon #27
base: master
Are you sure you want to change the base?
Conversation
bb78a4e
to
1684b95
Compare
The test runs for a long time, so it might be a good idea to split it into a separate Workflow |
TL;DR: I believe this would require an API break to implement. Let's ignore This in contrast to the API from xkbcommon which gets a buffer as its argument and writes the string into that. Thus, no |
Can't the static str get generated dynamically? Something like this:
The values of the RESULT buffer would have to get calculated. I guess the (big) downside of this is that there is more and more static memory getting allocated and never freed |
This would require a massive table for computation that would surely bloat the program. Just returning names like this is enough; we don't have to be concerned about the entirety of Unicode. We could also add a new method that handles Unicode, but returns For the tests, try running the test cases in parallel with |
That |
e33da9b
to
14645c4
Compare
Okay, returning names for keysyms representing Unicode codepoints will not be part of this PR. I used rayon for the test. The tests are failing right now, because the newest keysyms were not yet added to this crate. Once #26 gets merged, it should succeed. This will happen on a regular basis, but the output of the test lists the differences. The test is ignored per default and does not run in the CI workflow, because running the test still takes around 5 mins. Unfortunately we have to install xkbcommon even if the test is ignored. This adds around 2 min to the time the workflow takes to complete. I think the PR is ready to be reviewed |
Please rebase on #26 |
14645c4
to
94d6112
Compare
Sure, it has been rebased |
Fixes #18.
This PR changes the names of the named keysyms to get them in line with the names that
xkbcommon
returns and tools such asxdotool
expect. The only change needed for named keysyms was to get rid of theXK_
part. The only part I am struggling with, is returning the correct name for keysyms representing unnamed unicode codepoints. I have no experience with Rustno_std
and don't know how to implement it. The corresponding C code is very simple, so I left a placeholder and left this PR in a "Draft" state. Hopefully somebody can fill in the correct Rust code for it.I also added a test to make sure the function continues to return the same names for all possible keysyms. There are a few commented out parts that I needed to try the test on my machine with a newer libxkbcommon. I will remove them once the placeholder is properly implemented
I added the KEYSYM_MAX constant. It was added in xkbcommon 1.6.0 and shortens the time the test runs.