patch 8.1.2134: modifier keys are not always recognized

Problem:    Modifier keys are not always recognized.
Solution:   Handle key codes generated by xterm with modifyOtherKeys set.
            Add this to libvterm so we can debug it.

src/getchar.c

@@ -1733,6 +1733,25 @@ vgetc(void)
 		case K_XRIGHT:	c = K_RIGHT; break;
 	    }
 
+	    if (!no_reduce_keys)
+	    {
+		// A modifier was not used for a mapping, apply it to ASCII
+		// keys.
+		if ((mod_mask & MOD_MASK_CTRL)
+			&& ((c >= '`' && c <= 0x7f)
+			    || (c >= '@' && c <= '_')))
+		{
+		    c &= 0x1f;
+		    mod_mask &= ~MOD_MASK_CTRL;
+		}
+		if ((mod_mask & (MOD_MASK_META | MOD_MASK_ALT))
+			&& c >= 0 && c <= 127)
+		{
+		    c += 0x80;
+		    mod_mask &= ~MOD_MASK_META;
+		}
+	    }
+
 	    // For a multi-byte character get all the bytes and return the
 	    // converted character.
 	    // Note: This will loop until enough bytes are received!

src/globals.h

@@ -1006,6 +1006,8 @@ EXTERN int no_mapping INIT(= FALSE);	// currently no mapping allowed
 EXTERN int no_zero_mapping INIT(= 0);	// mapping zero not allowed
 EXTERN int allow_keys INIT(= FALSE);	// allow key codes when no_mapping
 					// is set
+EXTERN int no_reduce_keys INIT(= FALSE);  // do not apply Ctrl, Shift and Alt
+					  // to the key
 EXTERN int no_u_sync INIT(= 0);		// Don't call u_sync()
 #ifdef FEAT_EVAL
 EXTERN int u_sync_once INIT(= 0);	// Call u_sync() once when evaluating

src/libvterm/include/vterm.h

@@ -200,6 +200,7 @@ size_t vterm_output_get_buffer_remaining(const VTerm *vt);
 
 size_t vterm_output_read(VTerm *vt, char *buffer, size_t len);
 
+int vterm_is_modify_other_keys(VTerm *vt);
 void vterm_keyboard_unichar(VTerm *vt, uint32_t c, VTermModifier mod);
 void vterm_keyboard_key(VTerm *vt, VTermKey key, VTermModifier mod);
 

src/libvterm/src/keyboard.c

@@ -4,10 +4,21 @@
 
 #include "utf8.h"
 
+int vterm_is_modify_other_keys(VTerm *vt)
+{
+  return vt->state->mode.modify_other_keys;
+}
+
+
 void vterm_keyboard_unichar(VTerm *vt, uint32_t c, VTermModifier mod)
 {
   int needs_CSIu;
 
+  if (vt->state->mode.modify_other_keys && mod != 0) {
+    vterm_push_output_sprintf_ctrl(vt, C1_CSI, "27;%d;%d~", mod+1, c);
+    return;
+  }
+
   // The shift modifier is never important for Unicode characters
   // apart from Space
   if(c != ' ')

src/libvterm/src/state.c

@@ -1334,6 +1334,11 @@ static int on_csi(const char *leader, const long args[], int argcount, const cha
     vterm_state_setpen(state, args, argcount);
     break;
 
+  case LEADER('>', 0x6d): // xterm resource modifyOtherKeys
+    if (argcount == 2 && args[0] == 4)
+      state->mode.modify_other_keys = args[1] == 2;
+    break;
+
   case 0x6e: // DSR - ECMA-48 8.3.35
   case LEADER('?', 0x6e): // DECDSR
     val = CSI_ARG_OR(args[0], 0);

src/libvterm/src/vterm_internal.h

@@ -124,6 +124,7 @@ struct VTermState
     unsigned int leftrightmargin:1;
     unsigned int bracketpaste:1;
     unsigned int report_focus:1;
+    unsigned int modify_other_keys:1;
   } mode;
 
   VTermEncodingInstance encoding[4], encoding_utf8;

src/term.c

@@ -4198,6 +4198,99 @@ is_mouse_topline(win_T *wp)
 }
 #endif
 
+/*
+ * Put "string[new_slen]" in typebuf, or in "buf[bufsize]" if "buf" is not NULL.
+ * Remove "slen" bytes.
+ * Returns FAIL for error.
+ */
+    static int
+put_string_in_typebuf(
+	int	offset,
+	int	slen,
+	char_u	*string,
+	int	new_slen,
+	char_u	*buf,
+	int	bufsize,
+	int	*buflen)
+{
+    int		extra = new_slen - slen;
+
+    string[new_slen] = NUL;
+    if (buf == NULL)
+    {
+	if (extra < 0)
+	    // remove matched chars, taking care of noremap
+	    del_typebuf(-extra, offset);
+	else if (extra > 0)
+	    // insert the extra space we need
+	    ins_typebuf(string + slen, REMAP_YES, offset, FALSE, FALSE);
+
+	// Careful: del_typebuf() and ins_typebuf() may have reallocated
+	// typebuf.tb_buf[]!
+	mch_memmove(typebuf.tb_buf + typebuf.tb_off + offset, string,
+							     (size_t)new_slen);
+    }
+    else
+    {
+	if (extra < 0)
+	    // remove matched characters
+	    mch_memmove(buf + offset, buf + offset - extra,
+					   (size_t)(*buflen + offset + extra));
+	else if (extra > 0)
+	{
+	    // Insert the extra space we need.  If there is insufficient
+	    // space return -1.
+	    if (*buflen + extra + new_slen >= bufsize)
+		return FAIL;
+	    mch_memmove(buf + offset + extra, buf + offset,
+						   (size_t)(*buflen - offset));
+	}
+	mch_memmove(buf + offset, string, (size_t)new_slen);
+	*buflen = *buflen + extra + new_slen;
+    }
+    return OK;
+}
+
+/*
+ * Decode a modifier number as xterm provides it into MOD_MASK bits.
+ */
+    static int
+decode_modifiers(int n)
+{
+    int	    code = n - 1;
+    int	    modifiers = 0;
+
+    if (code & 1)
+	modifiers |= MOD_MASK_SHIFT;
+    if (code & 2)
+	modifiers |= MOD_MASK_ALT;
+    if (code & 4)
+	modifiers |= MOD_MASK_CTRL;
+    if (code & 8)
+	modifiers |= MOD_MASK_META;
+    return modifiers;
+}
+
+    static int
+modifiers2keycode(int modifiers, int *key, char_u *string)
+{
+    int new_slen = 0;
+
+    if (modifiers != 0)
+    {
+	// Some keys have the modifier included.  Need to handle that here to
+	// make mappings work.
+	*key = simplify_key(*key, &modifiers);
+	if (modifiers != 0)
+	{
+	    string[new_slen++] = K_SPECIAL;
+	    string[new_slen++] = (int)KS_MODIFIER;
+	    string[new_slen++] = modifiers;
+	}
+    }
+    return new_slen;
+}
+
 /*
  * Check if typebuf.tb_buf[] contains a terminal key code.
  * Check from typebuf.tb_buf[typebuf.tb_off] to typebuf.tb_buf[typebuf.tb_off
@@ -4229,8 +4322,7 @@ check_termcode(
     int		modifiers;
     char_u	*modifiers_start = NULL;
     int		key;
-    int		new_slen;
+    int		new_slen;   // Length of what will replace the termcode
-    int		extra;
     char_u	string[MAX_KEY_CODE_LEN + 1];
     int		i, j;
     int		idx = 0;
@@ -4401,16 +4493,9 @@ check_termcode(
 
 			    modifiers_start = tp + slen - 2;
 
-			    /* Match!  Convert modifier bits. */
+			    // Match!  Convert modifier bits.
-			    n = atoi((char *)modifiers_start) - 1;
+			    n = atoi((char *)modifiers_start);
-			    if (n & 1)
+			    modifiers |= decode_modifiers(n);
-				modifiers |= MOD_MASK_SHIFT;
-			    if (n & 2)
-				modifiers |= MOD_MASK_ALT;
-			    if (n & 4)
-				modifiers |= MOD_MASK_CTRL;
-			    if (n & 8)
-				modifiers |= MOD_MASK_META;
 
 			    slen = j;
 			}
@@ -4751,9 +4836,32 @@ not_enough:
 			winpos_status.tr_progress = STATUS_GOT;
 		}
 
-		// TODO: key with modifier:
+		// Key with modifier:
 		//	{lead}27;{modifier};{key}~
 		//	{lead}{key};{modifier}u
+		else if ((arg[0] == 27 && argc == 3 && trail == '~')
+			|| (argc == 2 && trail == 'u'))
+		{
+		    if (trail == 'u')
+			key = arg[0];
+		    else
+			key = arg[2];
+
+		    // insert modifiers with KS_MODIFIER
+		    modifiers = decode_modifiers(arg[1]);
+		    new_slen = modifiers2keycode(modifiers, &key, string);
+		    slen = csi_len;
+
+		    if (has_mbyte)
+			new_slen += (*mb_char2bytes)(key, string + new_slen);
+		    else
+			string[new_slen++] = key;
+
+		    if (put_string_in_typebuf(offset, slen, string, new_slen,
+						 buf, bufsize, buflen) == FAIL)
+			return -1;
+		    return len + new_slen - slen + offset;
+		}
 
 		// else: Unknown CSI sequence.  We could drop it, but then the
 		// user can't create a map for it.
@@ -5138,19 +5246,7 @@ not_enough:
 	/*
 	 * Add any modifier codes to our string.
 	 */
-	new_slen = 0;		/* Length of what will replace the termcode */
+	new_slen = modifiers2keycode(modifiers, &key, string);
-	if (modifiers != 0)
-	{
-	    /* Some keys have the modifier included.  Need to handle that here
-	     * to make mappings work. */
-	    key = simplify_key(key, &modifiers);
-	    if (modifiers != 0)
-	    {
-		string[new_slen++] = K_SPECIAL;
-		string[new_slen++] = (int)KS_MODIFIER;
-		string[new_slen++] = modifiers;
-	    }
-	}
 
 	/* Finally, add the special key code to our string */
 	key_name[0] = KEY2TERMCAP0(key);
@@ -5176,43 +5272,10 @@ not_enough:
 	    string[new_slen++] = key_name[0];
 	    string[new_slen++] = key_name[1];
 	}
-	string[new_slen] = NUL;
+	if (put_string_in_typebuf(offset, slen, string, new_slen,
-	extra = new_slen - slen;
+						 buf, bufsize, buflen) == FAIL)
-	if (buf == NULL)
-	{
-	    if (extra < 0)
-		/* remove matched chars, taking care of noremap */
-		del_typebuf(-extra, offset);
-	    else if (extra > 0)
-		/* insert the extra space we need */
-		ins_typebuf(string + slen, REMAP_YES, offset, FALSE, FALSE);
-
-	    /*
-	     * Careful: del_typebuf() and ins_typebuf() may have reallocated
-	     * typebuf.tb_buf[]!
-	     */
-	    mch_memmove(typebuf.tb_buf + typebuf.tb_off + offset, string,
-							    (size_t)new_slen);
-	}
-	else
-	{
-	    if (extra < 0)
-		/* remove matched characters */
-		mch_memmove(buf + offset, buf + offset - extra,
-					   (size_t)(*buflen + offset + extra));
-	    else if (extra > 0)
-	    {
-		/* Insert the extra space we need.  If there is insufficient
-		 * space return -1. */
-		if (*buflen + extra + new_slen >= bufsize)
 	    return -1;
-		mch_memmove(buf + offset + extra, buf + offset,
+	return retval == 0 ? (len + new_slen - slen + offset) : retval;
-						   (size_t)(*buflen - offset));
-	    }
-	    mch_memmove(buf + offset, string, (size_t)new_slen);
-	    *buflen = *buflen + extra + new_slen;
-	}
-	return retval == 0 ? (len + extra + offset) : retval;
     }
 
 #ifdef FEAT_TERMRESPONSE

src/terminal.c

@@ -1371,11 +1371,13 @@ term_convert_key(term_T *term, int c, char *buf)
 				break;
     }
 
+    // add modifiers for the typed key
+    mod |= mod_mask;
+
     /*
      * Convert special keys to vterm keys:
      * - Write keys to vterm: vterm_keyboard_key()
      * - Write output to channel.
-     * TODO: use mod_mask
      */
     if (key != VTERM_KEY_NONE)
 	/* Special key, let vterm convert it. */
@@ -1902,15 +1904,21 @@ term_vgetc()
 {
     int c;
     int save_State = State;
+    int modify_other_keys =
+			  vterm_is_modify_other_keys(curbuf->b_term->tl_vterm);
 
     State = TERMINAL;
     got_int = FALSE;
 #ifdef MSWIN
     ctrl_break_was_pressed = FALSE;
 #endif
+    if (modify_other_keys)
+	++no_reduce_keys;
     c = vgetc();
     got_int = FALSE;
     State = save_State;
+    if (modify_other_keys)
+	--no_reduce_keys;
     return c;
 }
 
@@ -2255,6 +2263,7 @@ term_win_entered()
 terminal_loop(int blocking)
 {
     int		c;
+    int		raw_c;
     int		termwinkey = 0;
     int		ret;
 #ifdef UNIX
@@ -2307,6 +2316,13 @@ terminal_loop(int blocking)
 	if (c == K_IGNORE)
 	    continue;
 
+	// vgetc may not include CTRL in the key when modify_other_keys is set.
+	raw_c = c;
+	if ((mod_mask & MOD_MASK_CTRL)
+		&& ((c >= '`' && c <= 0x7f)
+		    || (c >= '@' && c <= '_')))
+	    c &= 0x1f;
+
 #ifdef UNIX
 	/*
 	 * The shell or another program may change the tty settings.  Getting
@@ -2417,7 +2433,7 @@ terminal_loop(int blocking)
 		c = wc;
 	}
 # endif
-	if (send_keys_to_term(curbuf->b_term, c, TRUE) != OK)
+	if (send_keys_to_term(curbuf->b_term, raw_c, TRUE) != OK)
 	{
 	    if (c == K_MOUSEMOVE)
 		/* We are sure to come back here, don't reset the cursor color

src/version.c

@@ -753,6 +753,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2134,
 /**/
     2133,
 /**/