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

Support key composing (i.e. dead keys) in Wayland driver #4296

Merged
merged 2 commits into from
Apr 13, 2021

Conversation

lacc97
Copy link
Contributor

@lacc97 lacc97 commented Apr 9, 2021

Based on an old patch (#2488) from the old Bugzilla issue tracker. Unfortunately, migration to Github erased the name so I can't properly attribute authorship.

Copy link
Collaborator

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

Excellent work on rebasing this! I dug into the web archive and unfortunately the only name I could dig up was "chw", no e-mail address or anything. It does look like this was taken from that Weston URL though, so I dunno...

In any case, this is just what I would have reviewed for the original patch.

src/video/wayland/SDL_waylandevents.c Outdated Show resolved Hide resolved
src/video/wayland/SDL_waylandevents.c Outdated Show resolved Hide resolved
src/video/wayland/SDL_waylandevents.c Outdated Show resolved Hide resolved
@sezero
Copy link
Contributor

sezero commented Apr 9, 2021

Original bugzilla entry is https://bugzilla.libsdl.org/show_bug.cgi?id=3687
https://web.archive.org/web/20210210031224/https://bugzilla.libsdl.org/show_bug.cgi?id=3687
says the user email name is 'chw' - I can't get anything more out of it.

Based on an old patch by chw from the old Bugzilla issue tracker.

Authored-by: chw
Copy link
Collaborator

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

Latest lgtm! That said I am absolutely clueless on how dead keys actually work, so as long as someone who knows what they're doing can verify that this is functional this is good to go.

Fixes #2488.

@lacc97
Copy link
Contributor Author

lacc97 commented Apr 9, 2021

Just for reference, here's the xkbcommon documentation on compose keys: https://xkbcommon.org/doc/current/group__compose.html

@flibitijibibo
Copy link
Collaborator

I tested this for a bit with my admittedly little knowledge of compose keys and was able to verify that this works as advertised. Should be good to merge.

Copy link
Collaborator

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

This got stomped on a bit by #4303, so it needs a quick rebase. Since we're having to rebase, figured I'd do one more pass for any possible nits.

Comment on lines +666 to +669
/*
* See https://blogs.s-osg.org/compose-key-support-weston/
* for further explanation on dead keys in Wayland.
*/
Copy link
Collaborator

@flibitijibibo flibitijibibo Apr 13, 2021

Choose a reason for hiding this comment

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

Looks like this link is pretty dead even on Wayback Machine, so we can probably replace this with the official documentation for dead keys.

Suggested change
/*
* See https://blogs.s-osg.org/compose-key-support-weston/
* for further explanation on dead keys in Wayland.
*/
/* See https://xkbcommon.org/doc/current/group__compose.html
* for further explanation on dead keys in Wayland.
*/

Comment on lines +672 to +675
if (!(locale = SDL_getenv("LC_ALL")))
if (!(locale = SDL_getenv("LC_CTYPE")))
if (!(locale = SDL_getenv("LANG")))
locale = "C";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!(locale = SDL_getenv("LC_ALL")))
if (!(locale = SDL_getenv("LC_CTYPE")))
if (!(locale = SDL_getenv("LANG")))
locale = "C";
if (!(locale = SDL_getenv("LC_ALL"))) {
if (!(locale = SDL_getenv("LC_CTYPE"))) {
if (!(locale = SDL_getenv("LANG"))) {
locale = "C";
}
}
}

@slouken slouken merged commit 5c78df9 into libsdl-org:main Apr 13, 2021
@lacc97 lacc97 deleted the xkb-compose branch April 14, 2021 00:56
joolswills pushed a commit to RetroPie/SDL that referenced this pull request May 22, 2021
…4296)

Based on an old patch by chw from the old Bugzilla issue tracker.

Authored-by: chw

Co-authored-by: Sam Lantinga <[email protected]>
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.

4 participants