Re: [PATCH weston v2] clients: Add XKB compose key support
On Fri, Oct 07, 2016 at 12:56:00PM -0700, Bryce Harrington wrote:
> On Fri, Oct 07, 2016 at 02:48:41PM +0300, Ran Benita wrote:
> > > + /* Look up the appropriate locale, or use "C" as default */
> > > + locale = getenv("LC_ALL");
> > > + if (!locale)
> > > + locale = "C";
> >
> > Is there a reason why you decided not to use the "full" procedure, i.e.
> > also try LC_CTYPE and LANG?
>
> No particular reason. I did wonder if it would be worthwhile to check
> those too, but wasn't sure what order they should be checked. Should it
> be LANG first, then LC_CTYPE, and then fallback to LC_ALL?
I got the order from locale(7):
If the second argument to setlocale(3) is an empty string, "",
for the default locale, it is determined using the following
steps:
1. If there is a non-null environment variable LC_ALL, the
value of LC_ALL is used.
2. If an environment variable with the same name as one of
the categories above exists and is non-null, its value is
used for that category.
3. If there is a non-null environment variable LANG, the
value of LANG is used.
at step 2, the relevant category in this case is LC_CTYPE. So
LC_ALL -> LC_CTYPE -> LANG -> "C".
Ran
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients: Add XKB compose key support
On Fri, Oct 07, 2016 at 02:48:41PM +0300, Ran Benita wrote:
> > + /* Look up the appropriate locale, or use "C" as default */
> > + locale = getenv("LC_ALL");
> > + if (!locale)
> > + locale = "C";
>
> Is there a reason why you decided not to use the "full" procedure, i.e.
> also try LC_CTYPE and LANG?
No particular reason. I did wonder if it would be worthwhile to check
those too, but wasn't sure what order they should be checked. Should it
be LANG first, then LC_CTYPE, and then fallback to LC_ALL?
> > + /* Set up XKB compose table */
> > + compose_table =
> > xkb_compose_table_new_from_locale(input->display->xkb_context,
> > + locale,
> > +
> > XKB_COMPOSE_COMPILE_NO_FLAGS);
> > + if (!compose_table) {
> > + fprintf(stderr, "could not create XKB compose table for locale
> > '%s'\n", locale);
> > + xkb_state_unref(state);
> > + xkb_keymap_unref(keymap);
> > + return;
> > + }
>
> In my opinion, it would be better to proceed without compose, than to
> fail the entire setup. If, for example, the compose files are not
> available, then it's OK to just skip it. The alternative is that the
> window does not show up at all (I assume).
>
> Ran
Ok, good point.
Bryce
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients: Add XKB compose key support
> + /* Look up the appropriate locale, or use "C" as default */
> + locale = getenv("LC_ALL");
> + if (!locale)
> + locale = "C";
Is there a reason why you decided not to use the "full" procedure, i.e.
also try LC_CTYPE and LANG?
> + /* Set up XKB compose table */
> + compose_table =
> xkb_compose_table_new_from_locale(input->display->xkb_context,
> + locale,
> +
> XKB_COMPOSE_COMPILE_NO_FLAGS);
> + if (!compose_table) {
> + fprintf(stderr, "could not create XKB compose table for locale
> '%s'\n", locale);
> + xkb_state_unref(state);
> + xkb_keymap_unref(keymap);
> + return;
> + }
In my opinion, it would be better to proceed without compose, than to
fail the entire setup. If, for example, the compose files are not
available, then it's OK to just skip it. The alternative is that the
window does not show up at all (I assume).
Ran
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients: Add XKB compose key support
Hi,
On 7 October 2016 at 05:24, Bryce Harrington wrote:
> @@ -3016,8 +3024,38 @@ keyboard_handle_keymap(void *data, struct wl_keyboard
> *keyboard,
> return;
> }
>
> + /* Look up the appropriate locale, or use "C" as default */
> + locale = getenv("LC_ALL");
> + if (!locale)
> + locale = "C";
> +
> + /* Set up XKB compose table */
> + compose_table =
> xkb_compose_table_new_from_locale(input->display->xkb_context,
> + locale,
> +
> XKB_COMPOSE_COMPILE_NO_FLAGS);
This line is quite long. You can get it to exactly 80 characters like
so (this style is quite common within Weston):
compose_table =
xkb_compose_table_new_from_locale(input->display->xkb_context,
locale,
XKB_COMPOSE_COMPILE_NO_FLAGS);
> +/* Translate symbols appropriately if a compose sequence is being entered */
> +static xkb_keysym_t
> +process_key_press(xkb_keysym_t sym, struct input *input)
> +{
> + if (sym == XKB_KEY_NoSymbol)
> + return sym;
> + if (xkb_compose_state_feed(input->xkb.compose_state, sym) !=
> XKB_COMPOSE_FEED_ACCEPTED)
> + return sym;
Same here.
> + switch (xkb_compose_state_get_status(input->xkb.compose_state)) {
> + case XKB_COMPOSE_COMPOSING:
> + return XKB_KEY_NoSymbol;
> + case XKB_COMPOSE_COMPOSED:
> + return
> xkb_compose_state_get_one_sym(input->xkb.compose_state);
> + case XKB_COMPOSE_CANCELLED:
> + return XKB_KEY_NoSymbol;
> + case XKB_COMPOSE_NOTHING:
> + return sym;
> + }
> +
> + return sym;
This line is unreachable, and could also be handled with a 'default' case.
The rest looks good to me though, and I like the split function, so
with those fixed:
Reviewed-by: Daniel Stone
Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] clients: Add XKB compose key support
This adds single-symbol compose support using libxkbcommon's compose
functionality. E.g., assuming you have the right alt key defined as
your compose key, typing +i+' will produce í, and +y+= will
produce ¥. This makes compose key work for weston-editor,
weston-terminal, weston-eventdemo, and any other clients that use
Weston's window.* routines for accepting and managing keyboard input.
Compose sequences are loaded from the system's standard tables. As
well, libxkbcommon will transparently load custom sequences from the
user's ~/.XCompose file.
Note that due to limitations in toytoolkit's key handler interface, only
compose sequences resulting in single symbols are supported. While
libxkbcommon supports multi-symbol compose strings, support for passing
text buffers to Weston clients is left as future work.
This largely obviates the need for the weston-simple-im input method
client, which had provided a very limited compose functionality that was
only available in clients implementing the zwp_input_method protocol,
and with no mechanism to load system or user-specified compose keys.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=53648
Signed-off-by: Bryce Harrington
---
clients/window.c | 64
1 file changed, 64 insertions(+)
diff --git a/clients/window.c b/clients/window.c
index 216ef96..0110ae4 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -63,6 +63,7 @@ typedef void *EGLContext;
#endif /* no HAVE_CAIRO_EGL */
#include
+#include
#include
#include
@@ -372,6 +373,8 @@ struct input {
struct {
struct xkb_keymap *keymap;
struct xkb_state *state;
+ struct xkb_compose_table *compose_table;
+ struct xkb_compose_state *compose_state;
xkb_mod_mask_t control_mask;
xkb_mod_mask_t alt_mask;
xkb_mod_mask_t shift_mask;
@@ -2979,6 +2982,9 @@ keyboard_handle_keymap(void *data, struct wl_keyboard
*keyboard,
struct input *input = data;
struct xkb_keymap *keymap;
struct xkb_state *state;
+ struct xkb_compose_table *compose_table;
+ struct xkb_compose_state *compose_state;
+ char *locale;
char *map_str;
if (!data) {
@@ -2997,6 +3003,7 @@ keyboard_handle_keymap(void *data, struct wl_keyboard
*keyboard,
return;
}
+ /* Set up XKB keymap */
keymap = xkb_keymap_new_from_string(input->display->xkb_context,
map_str,
XKB_KEYMAP_FORMAT_TEXT_V1,
@@ -3009,6 +3016,7 @@ keyboard_handle_keymap(void *data, struct wl_keyboard
*keyboard,
return;
}
+ /* Set up XKB state */
state = xkb_state_new(keymap);
if (!state) {
fprintf(stderr, "failed to create XKB state\n");
@@ -3016,8 +3024,38 @@ keyboard_handle_keymap(void *data, struct wl_keyboard
*keyboard,
return;
}
+ /* Look up the appropriate locale, or use "C" as default */
+ locale = getenv("LC_ALL");
+ if (!locale)
+ locale = "C";
+
+ /* Set up XKB compose table */
+ compose_table =
xkb_compose_table_new_from_locale(input->display->xkb_context,
+ locale,
+
XKB_COMPOSE_COMPILE_NO_FLAGS);
+ if (!compose_table) {
+ fprintf(stderr, "could not create XKB compose table for locale
'%s'\n", locale);
+ xkb_state_unref(state);
+ xkb_keymap_unref(keymap);
+ return;
+ }
+
+ /* Set up XKB compose state */
+ compose_state = xkb_compose_state_new(compose_table,
+ XKB_COMPOSE_STATE_NO_FLAGS);
+ if (!compose_state) {
+ fprintf(stderr, "could not create XKB compose state\n");
+ xkb_compose_table_unref(compose_table);
+ xkb_state_unref(state);
+ xkb_keymap_unref(keymap);
+ }
+
+ xkb_compose_state_unref(input->xkb.compose_state);
+ xkb_compose_table_unref(input->xkb.compose_table);
xkb_keymap_unref(input->xkb.keymap);
xkb_state_unref(input->xkb.state);
+ input->xkb.compose_state = compose_state;
+ input->xkb.compose_table = compose_table;
input->xkb.keymap = keymap;
input->xkb.state = state;
@@ -3056,6 +3094,29 @@ keyboard_handle_leave(void *data, struct wl_keyboard
*keyboard,
input_remove_keyboard_focus(input);
}
+/* Translate symbols appropriately if a compose sequence is being entered */
+static xkb_keysym_t
+process_key_press(xkb_keysym_t sym, struct input *input)
+{
+ if (sym == XKB_KEY_NoSymbol)
+ return sym;
+ if (xkb_compose_state_feed(input->xkb.compose_state, sym) !=
XKB_COMPOSE_F
