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

Jump to bottom

Add Keysym::NoSymbol #17

Merged
merged 2 commits into from
Oct 22, 2023
Merged

Add Keysym::NoSymbol #17

merged 2 commits into from
Oct 22, 2023

Conversation

pentamassiv
Copy link
Contributor

Fixes #16

@pentamassiv
Copy link
Contributor Author

pentamassiv commented Oct 5, 2023 •
edited
Loading

I ran the keysym-generator locally to see the changes. It worked as expected, but it also added a number of unrelated keysyms, which is why I excluded this change.
Should I add another commit with those changes? Then the tests will pass too.

notgull
notgull requested changes Oct 5, 2023
View reviewed changes
Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

What OS are you running? If Keysyms are only added and not removed then it should be fine.

src/lib.rs Outdated Show resolved Hide resolved
@pentamassiv
Copy link
Contributor Author

I am running Arch Linux. I pushed the commit so you can have a look. There are also four removals

@notgull
Copy link
Member

notgull commented Oct 22, 2023

I am running Arch Linux. I pushed the commit so you can have a look. There are also four removals

We should probably have a standard "generate Keysyms" script that doesn't depend on the host OS. Could probably just use Docker or something to run it inside of Ubuntu.

@pentamassiv
Copy link
Contributor Author

Now that you created #26, I regenerated the keysyms with your suggested command. Should be good to merge now, right?

@pentamassiv pentamassiv requested a review from notgull October 22, 2023 22:11
@pentamassiv pentamassiv changed the title Rename and move NO_SYMBOL to Keysym::NoSymbol Add Keysym::NoSymbol Oct 22, 2023
notgull
notgull approved these changes Oct 22, 2023
View reviewed changes
Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Thanks!

@notgull notgull merged commit b705222 into rust-windowing:master Oct 22, 2023
@pentamassiv pentamassiv deleted the no_symbol branch October 22, 2023 22:14
@notgull notgull mentioned this pull request Jun 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notgull notgull notgull approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

xkeysym::NO_SYMBOL not part of xkeysym::key
2 participants
@pentamassiv @notgull