irssi-import / bugs.irssi.org

bugs.irssi.org archive
https://github.com/irssi/irssi/issues
0 stars 0 forks source link

can't bind accented characters #553

Open irssibot opened 16 years ago

irssibot commented 16 years ago

Accented characters (like é, è, à, ù) are not recognized in bindings. This is probably due to the fact irssi is waiting for a 8 bits char while those are 16+ bits in UTF-8 encoding.

If '/bind' is used to list all the bindings, all those which are composed of accented characters are actually shifted from one space to the left (This is what me think it's a encoding issue)

meta-1           change_window 1
meta-é          change_window 2
meta-4           change_window 4

Here is the relevant part of my xorg config concerning keyboard:

    Identifier     "Generic Keyboard"
    Driver         "kbd"
    Option         "CoreKeyboard"
    Option         "XkbRules" "xorg"
    Option         "XkbModel" "pc105"
    Option         "XkbLayout" "fr"
--
irssibot commented 15 years ago

any information about this bug?

I discover that if I set term_charset to something else than utf-8 I'm able to use accentuated chars

irssibot commented 15 years ago

Exactly the same bug for me...

I'll add that it I set my term in ISO : /set term_charset ISO-8859-1 And then /bind meta-é change_window 2 ...it works.

But actually my term_charset is UTF-8 (system-wide) and it doesn't work.

irssibot commented 15 years ago

I can also confirm this bug. bind meta+£ does not work.

irssibot commented 15 years ago

Confirmed. UTF-8 multibyte characters do not work in key bindings, for example meta-ö, meta-ä and meta-shift-4 cannot be bound on Scandinavian keyboard layouts.

irssibot commented 15 years ago

Same problem here on windows with cygwin&puttycyg.

irssibot commented 14 years ago

The following modification made it work for me. If this patch seems reasonable for you, please use it.

--- src/fe-common/core/keyboard.c       (revision 5140)
+++ src/fe-common/core/keyboard.c       (working copy)
@@ -569,8 +569,15 @@
        }

         first_key = keyboard->key_state == NULL;
-       combo = keyboard->key_state == NULL ? g_strdup(key) :
-                g_strconcat(keyboard->key_state, "-", key, NULL);
+       if (keyboard->key_state == NULL) {
+               combo = g_strdup(key);
+       } else if (g_utf8_validate(key, 2, NULL)) {
+               char wchar[256];
+               g_snprintf(wchar, sizeof(wchar), "%c-%c", key[0], key[1]);
+               combo = g_strconcat(keyboard->key_state, "-", wchar, NULL);
+       } else {
+               combo = g_strconcat(keyboard->key_state, "-", key, NULL);
+       }
        g_free_and_null(keyboard->key_state);

        rec = g_tree_search(key_states,
irssibot commented 14 years ago

Same problem here (Linux, rxvt-unicode). This patch fixes the issue.

irssibot commented 13 years ago

So Rhonda put it into Debian, and here is what happened: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=625690

I guess this patch needs refinement :(

irssibot commented 13 years ago

Heh, so it only took more than a year to find out consequencies of proposed patch :)

I myself never used Alt(Esc)+arrows, but i confirm that the first patch have broken it.

Anyway, attached is another version of patch, so far i don't face any problem with that (utf-8 bindings work, alt+left-right arrows work).

Btw, alt+up/down doesn't work for me with our without any patch (guess it's my terminal or according irssi binding is missing by default).

irssibot commented 13 years ago

irssi-553.patch

Index: src/fe-common/core/keyboard.c
===================================================================
--- src/fe-common/core/keyboard.c   (revision 5205)
+++ src/fe-common/core/keyboard.c   (working copy)
@@ -569,8 +569,15 @@
    }

         first_key = keyboard->key_state == NULL;
-   combo = keyboard->key_state == NULL ? g_strdup(key) :
-                g_strconcat(keyboard->key_state, "-", key, NULL);
+   if (keyboard->key_state == NULL) {
+       combo = g_strdup(key);
+   } else if (key[1] != '\0') {
+       char wchar[256];
+       g_snprintf(wchar, sizeof(wchar), "%c-%c", key[0], key[1]);
+       combo = g_strconcat(keyboard->key_state, "-", wchar, NULL);
+   } else {
+       combo = g_strconcat(keyboard->key_state, "-", key, NULL);
+   }
    g_free_and_null(keyboard->key_state);

    rec = g_tree_search(key_states,
irssibot commented 13 years ago

Heh, so it only took more than a year to find out consequencies of proposed patch :)

I myself never used Alt(Esc)+arrows, but i confirm that the first patch have broken it.

Anyway, attached is another version of patch, so far i don't face any problem with that (utf-8 bindings work, alt+left-right arrows work).

Btw, alt+up/down doesn't work for me with our without any patch (guess it's my terminal or according irssi binding is missing by default).

irssibot commented 13 years ago

irssi-553.patch

Index: src/fe-common/core/keyboard.c
===================================================================
--- src/fe-common/core/keyboard.c   (revision 5205)
+++ src/fe-common/core/keyboard.c   (working copy)
@@ -569,8 +569,15 @@
    }

         first_key = keyboard->key_state == NULL;
-   combo = keyboard->key_state == NULL ? g_strdup(key) :
-                g_strconcat(keyboard->key_state, "-", key, NULL);
+   if (keyboard->key_state == NULL) {
+       combo = g_strdup(key);
+   } else if (key[1] != '\0') {
+       char wchar[256];
+       g_snprintf(wchar, sizeof(wchar), "%c-%c", key[0], key[1]);
+       combo = g_strconcat(keyboard->key_state, "-", wchar, NULL);
+   } else {
+       combo = g_strconcat(keyboard->key_state, "-", key, NULL);
+   }
    g_free_and_null(keyboard->key_state);

    rec = g_tree_search(key_states,
irssibot commented 13 years ago

Someone was right, it also depends on terminal if the patch affects alt+arrows.. Let's see if this patch works better (tested in gnome-terminal and putty).

irssibot commented 13 years ago

irssi-553.patch

Index: src/fe-common/core/keyboard.c
===================================================================
--- src/fe-common/core/keyboard.c   (revision 5205)
+++ src/fe-common/core/keyboard.c   (working copy)
@@ -569,8 +569,15 @@
    }

         first_key = keyboard->key_state == NULL;
-   combo = keyboard->key_state == NULL ? g_strdup(key) :
-                g_strconcat(keyboard->key_state, "-", key, NULL);
+   if (keyboard->key_state == NULL) {
+       combo = g_strdup(key);
+   } else if (key[1] != '\0' && key[1] != '[') {
+       char wchar[256];
+       g_snprintf(wchar, sizeof(wchar), "%c-%c", key[0], key[1]);
+       combo = g_strconcat(keyboard->key_state, "-", wchar, NULL);
+   } else {
+       combo = g_strconcat(keyboard->key_state, "-", key, NULL);
+   }
    g_free_and_null(keyboard->key_state);

    rec = g_tree_search(key_states,
irssibot commented 13 years ago

Debian (and ubuntu) seem to be applying this alternate patch now. Unfortunately it breaks alt+backspace for me (which used to delete the word to the left of the cursor) using my usual irss in tmux through ssh in rxvt-unicode setup (all using utf-8). Specifically it turns what used to be "^[-^?" into "^[-^-?".

I have created an alternate patch which leaves alt+backspace working, and might also fix the original problem, but I've not yet played around with remapping my keyboard enough to be able to test that. It's based on a guess at what the first patch was trying to do: fix up multibyte utf-8 input (and only that) to match the table used for keybindings. But instead of trying to list multibyte inputs that should not be munged (as the updated patch tries) this one checks if the input is valid utf-8 (like the original patch did) and a single character (the original patch did not have that check, causing it to accept multiple ascii characters as seen in encoded escaped characters).

Oh, and I just realized this patch is still wrong for utf-8 sequences longer than one byte (but so were the previous patches).

Does this approach make sense?

irssibot commented 13 years ago

18bind_utf8-fix

=== modified file 'src/fe-common/core/keyboard.c'
--- src/fe-common/core/keyboard.c   2011-10-13 20:10:30 +0000
+++ src/fe-common/core/keyboard.c   2011-10-13 20:17:39 +0000
@@ -569,8 +569,22 @@
    }

         first_key = keyboard->key_state == NULL;
-   combo = keyboard->key_state == NULL ? g_strdup(key) :
-                g_strconcat(keyboard->key_state, "-", key, NULL);
+   /* fix for binding utf8 characters, done by kyak, upstream bug #553 */
+   if (keyboard->key_state == NULL) {
+       combo = g_strdup(key);
+   } else if (g_utf8_validate(key, -1, NULL) &&
+          g_utf8_next_char(key) == '\0') {
+       /* We are looking at a single utf-8 character.
+        * Munge it so utf8 keybindings work.
+        * (we need to not hit this for things like alt+arrow
+        * or alt+enter, or those keybindings break).
+        */
+       char wchar[256];
+       g_snprintf(wchar, sizeof(wchar), "%c-%c", key[0], key[1]);
+       combo = g_strconcat(keyboard->key_state, "-", wchar, NULL);
+   } else {
+       combo = g_strconcat(keyboard->key_state, "-", key, NULL);
+   }
    g_free_and_null(keyboard->key_state);

    rec = g_tree_search(key_states,
irssibot commented 13 years ago

My previously attached patch was crap. Please ignore it.

I thought about this some more, and I think it makes more sense to add multibyte utf-8 sequences to keystates correctly (that is: with the utf-8 sequence intact instead of broken up with '-' characters). The final "else" case in expand_key (which is what's hit for these keys) just seems to assume a unibyte encoding when it inserts those '-'s.

So here's an ugly patch that picks up utf-8 in expand_key and preserves it. This time I actually tested not just that alt+backspace still works but also that alt+eurosign can now be bound, and it seems to do the trick.

Thoughts?

irssibot commented 13 years ago

18bind_utf8-fix

=== modified file 'irssi-version.h'
--- irssi-version.h 2011-10-13 20:10:30 +0000
+++ irssi-version.h 2011-10-14 19:13:07 +0000
@@ -1,2 +1,2 @@
 #define IRSSI_VERSION_DATE 20100403
-#define IRSSI_VERSION_TIME 1817
+#define IRSSI_VERSION_TIME 1617

=== modified file 'src/fe-common/core/keyboard.c'
--- src/fe-common/core/keyboard.c   2011-10-13 20:10:30 +0000
+++ src/fe-common/core/keyboard.c   2011-10-14 21:14:28 +0000
@@ -304,6 +304,20 @@
                         /* possibly beginning of keycombo */
            start = key;
                         last_hyphen = FALSE;
+       } else if (g_utf8_validate(key, -1, NULL)) {
+           /* Assume we are looking at the start of a
+            * multibyte sequence we will receive as-is,
+            * so add it to the list as-is.
+            */
+           const char *p, *end = g_utf8_next_char(key);
+           for (p = key; p != end; p++)
+               expand_out_char(*out, *p);
+           expand_out_char(*out, '-');
+           /* The for loop skips past the remaining character.
+            * Nasty, I know...
+            */
+           key = end - 1;
+           last_hyphen = FALSE;
        } else {
            expand_out_char(*out, *key);
            expand_out_char(*out, '-');