From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Third Newsgroups: gmane.emacs.devel Subject: Re: Emacs 26 MacOS bugs Date: Wed, 7 Feb 2018 22:55:05 +0000 Message-ID: <20180207225505.GB1066@breton.holly.idiocy.org> References: <20180206190518.f43jsoughhlsi22y@adversary.org> <20180206222722.5uf43fd56p44dk5z@adversary.org> <20180207004331.q4cll6nyldse2bin@adversary.org> <20180207021301.zpvpexdkewztqxo4@adversary.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="mP3DRpeJDSE+ciuQ" Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1518044011 23266 195.159.176.226 (7 Feb 2018 22:53:31 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 7 Feb 2018 22:53:31 +0000 (UTC) User-Agent: Mutt/1.9.3 (2018-01-21) Cc: Philipp Stephani , Emacs developers , Noam Postavsky To: Ben McGinnes Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Feb 07 23:53:26 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ejYaZ-00052h-IV for ged-emacs-devel@m.gmane.org; Wed, 07 Feb 2018 23:53:15 +0100 Original-Received: from localhost ([::1]:40436 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ejYca-0006dL-Vn for ged-emacs-devel@m.gmane.org; Wed, 07 Feb 2018 17:55:21 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37133) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ejYcR-0006br-MV for emacs-devel@gnu.org; Wed, 07 Feb 2018 17:55:14 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ejYcQ-0004Dy-4v for emacs-devel@gnu.org; Wed, 07 Feb 2018 17:55:11 -0500 Original-Received: from mail-wr0-x229.google.com ([2a00:1450:400c:c0c::229]:35289) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ejYcP-0004DH-Mg for emacs-devel@gnu.org; Wed, 07 Feb 2018 17:55:10 -0500 Original-Received: by mail-wr0-x229.google.com with SMTP id w50so2809435wrc.2 for ; Wed, 07 Feb 2018 14:55:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=idEjsKl1sllCU2dNg9pBEE8WmMOIfn+xmJkbKUx6ei0=; b=VoZ7L8iUGT01eb2kQvJ3Jh/6XcSGHCNo2R6sIFPCqZThbUQJ2wB9DXly1IaBViFT9s JbVkQYYA1ICUKq5WSWdvHM0+ZuKhE14xckgQfP0GdVbdSnrmqHc28msVW8cZBuE+tFgJ j81krvPtGmCOS4e2q8xuJT39Sjh0Hs3jvp/fH6U5mbvfhJMi/BcADgAjwkGvfvSfimia ll8Cg9coUu/oQzQu0x1oLrg/0SWpXeaT23LO8XXyoSVSE5bhaqxwIADv6/W/npgtKgzU 7Z37T6iLVjqXohJe+iwgwB3FrDY7O2ClfEZhi6QACcK2nrG0bi+MNo8KbtwOtJ7OC9sp EIhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=idEjsKl1sllCU2dNg9pBEE8WmMOIfn+xmJkbKUx6ei0=; b=FxUKS4S3MpPfZE9BnYYQv/KrDAiPE6zeKAK4bZ5nxRsGhkST0r4yE2vRyJ4oseGGO+ etFq7ElF3dLdAKpf+Rptwzv2QjaBwKYpXcaD1wLnYf3P7sEBLyM/kqZqWuwCEzXqJVuE 0H7AJZRQWDbAUJB84WeL05vKJ3j5QbZkvJAW5pat8ag3dhyQltiarBv/Sb8fjHydnKad 9yq950BonbHWG8v8WJa/F7B6Ax8FX0ftDZhcG4HMsTa4PR3Rr8rrCuWC2u9QBS9T+biw XbH6o6ex+ZSy3S1fqcp66OZVi1ieHvawHKWPs5DhDPIs6ORL4DElcQes/qCdJT0jkNWu v7dw== X-Gm-Message-State: APf1xPCPSgal9jfSuAJz30mHMb+1mwbUOBslb5VGFf/cnnrHKuQ43vu4 RY5ey4XmduCoEgkYjLhIV6k= X-Google-Smtp-Source: AH8x227UJX+vHd1XzaXd/qeEv0wCTY+ookorPo8lNdohHg9GBwMp3CtvNrfgTClXBtAaOTUZvRucng== X-Received: by 10.223.156.145 with SMTP id d17mr7421314wre.1.1518044108471; Wed, 07 Feb 2018 14:55:08 -0800 (PST) Original-Received: from breton.holly.idiocy.org (ip6-2001-08b0-03f8-8129-404c-cfbb-5826-6c38.holly.idiocy.org. [2001:8b0:3f8:8129:404c:cfbb:5826:6c38]) by smtp.gmail.com with ESMTPSA id b65sm3484881wrd.26.2018.02.07.14.55.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Feb 2018 14:55:07 -0800 (PST) Content-Disposition: inline In-Reply-To: <20180207021301.zpvpexdkewztqxo4@adversary.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:400c:c0c::229 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:222620 Archived-At: --mP3DRpeJDSE+ciuQ Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Wed, Feb 07, 2018 at 01:13:01PM +1100, Ben McGinnes wrote: > On Tue, Feb 06, 2018 at 08:33:31PM -0500, Noam Postavsky wrote: > > AFAIK, there's no firm date, but emacs-26 is in feature freeze and > > only taking very safe/urgent bug fixes. So it's possible these bugs > > won't be fixed until after 26.1 > > I think I could make a very convincing argument regarding the modifier > keys here being urgent. If it was only rendering the hyper key > useless, then maybe you could argue it as not being urgent, but it's > not merely that ... The good news is that the patch that affects the modifier keys isn’t in Emacs 26, it was pushed to master only. > any attempt to assign the hyper function to any > other key (e.g. right alt), just adds the hyper function instead of > replacing the previous function with that. So if left alt is meta and > you set right alt as hyper, it just becomes a combined hyper-meta. > It's no longer possible to set the hyper key on its own. > > Which means the code commentary about changing the nil to none for all > those ns modifier keys is, in fact, completely wrong. You *do* want nil > there because if you're re-assigning the key to perform another task > you actually do want to clear the original setting out of it. > > As for a stable fix, that's dead simple, roll back everything from the > 2016 commit that was merged into master on the 5th and it's done. I know you think there was nothing wrong with the old code, but it was broken in various ways that weren’t obvious how to fix. The new code should behave more consistently, once we clear up the bugs you’ve found. Can you please try the attached patch? Philipp, it seems that EV_MODIFIERS2 was quite, quite broken. I’ve split it into various parts and tried to fix it. Additionally the fn key has some strange interactions with ‘function’ keys, like the Fx keys, arrows, tab and so on. The old code simply didn’t set its modifier when those keys were used, so I’ve followed that lead and am XORing it out of emacs_event->modifiers. -- Alan Third --mP3DRpeJDSE+ciuQ Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename="0001-Fix-modifier-keys-on-NS-port.patch" Content-Transfer-Encoding: 8bit >From b089831704c91fbd0bf0baa45ba6f0c78cf06e41 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Wed, 7 Feb 2018 22:39:17 +0000 Subject: [PATCH] Fix modifier keys on NS port * src/nsterm.m (EV_MODIFIERS_OPTION): (EV_MODIFIERS_CONTROL): (EV_MODIFIERS_COMMAND): New macros. (EV_MODIFIERS2): Split out into new macros to make it easier to understand. (EmacsView::keyDown): Remove duplicate functionality and handle fn key correctly. --- src/nsterm.m | 83 ++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/src/nsterm.m b/src/nsterm.m index b7f5a32c09..58ebca4153 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -352,31 +352,45 @@ - (NSColor *)colorUsingDefaultColorSpace #define NSRightCommandKeyMask (0x000010 | NSEventModifierFlagCommand) #define NSLeftAlternateKeyMask (0x000020 | NSEventModifierFlagOption) #define NSRightAlternateKeyMask (0x000040 | NSEventModifierFlagOption) -#define EV_MODIFIERS2(flags) \ - (((flags & NSEventModifierFlagHelp) ? \ - hyper_modifier : 0) \ - | (!EQ (ns_right_alternate_modifier, Qleft) && \ - ((flags & NSRightAlternateKeyMask) \ - == NSRightAlternateKeyMask) ? \ - parse_solitary_modifier (ns_right_alternate_modifier) : 0) \ - | ((flags & NSEventModifierFlagOption) ? \ - parse_solitary_modifier (ns_alternate_modifier) : 0) \ - | ((flags & NSEventModifierFlagShift) ? \ - shift_modifier : 0) \ - | (!EQ (ns_right_control_modifier, Qleft) && \ - ((flags & NSRightControlKeyMask) \ - == NSRightControlKeyMask) ? \ - parse_solitary_modifier (ns_right_control_modifier) : 0) \ - | ((flags & NSEventModifierFlagControl) ? \ - parse_solitary_modifier (ns_control_modifier) : 0) \ - | ((flags & NS_FUNCTION_KEY_MASK) ? \ - parse_solitary_modifier (ns_function_modifier) : 0) \ - | (!EQ (ns_right_command_modifier, Qleft) && \ - ((flags & NSRightCommandKeyMask) \ - == NSRightCommandKeyMask) ? \ - parse_solitary_modifier (ns_right_command_modifier) : 0) \ - | ((flags & NSEventModifierFlagCommand) ? \ - parse_solitary_modifier (ns_command_modifier):0)) + +#define EV_MODIFIERS_OPTION(flags) \ + (((flags & NSRightAlternateKeyMask) == NSRightAlternateKeyMask && \ + ! EQ (ns_right_alternate_modifier, Qleft)) ? \ + parse_solitary_modifier (ns_right_alternate_modifier) : 0) \ + | (((flags & NSLeftAlternateKeyMask) == NSLeftAlternateKeyMask || \ + (flags & NSRightAlternateKeyMask) == NSRightAlternateKeyMask && \ + EQ (ns_right_alternate_modifier, Qleft)) ? \ + parse_solitary_modifier (ns_alternate_modifier) : 0) + +#define EV_MODIFIERS_CONTROL(flags) \ + (((flags & NSRightControlKeyMask) == NSRightControlKeyMask && \ + ! EQ (ns_right_control_modifier, Qleft)) ? \ + parse_solitary_modifier (ns_right_control_modifier) : 0) \ + | (((flags & NSLeftControlKeyMask) == NSLeftControlKeyMask || \ + (flags & NSRightControlKeyMask) == NSRightControlKeyMask && \ + EQ (ns_right_control_modifier, Qleft)) ? \ + parse_solitary_modifier (ns_control_modifier) : 0) + +#define EV_MODIFIERS_COMMAND(flags) \ + (((flags & NSRightCommandKeyMask) == NSRightCommandKeyMask && \ + ! EQ (ns_right_command_modifier, Qleft)) ? \ + parse_solitary_modifier (ns_right_command_modifier) : 0) \ + | (((flags & NSLeftCommandKeyMask) == NSLeftCommandKeyMask || \ + (flags & NSRightCommandKeyMask) == NSRightCommandKeyMask && \ + EQ (ns_right_command_modifier, Qleft)) ? \ + parse_solitary_modifier (ns_command_modifier) : 0) + +#define EV_MODIFIERS2(flags) \ + (((flags & NSEventModifierFlagHelp) ? \ + hyper_modifier : 0) \ + | ((flags & NSEventModifierFlagShift) ? \ + shift_modifier : 0) \ + | ((flags & NS_FUNCTION_KEY_MASK) ? \ + parse_solitary_modifier (ns_function_modifier) : 0) \ + | EV_MODIFIERS_OPTION (flags) \ + | EV_MODIFIERS_CONTROL (flags) \ + | EV_MODIFIERS_COMMAND (flags)) + #define EV_MODIFIERS(e) EV_MODIFIERS2 ([e modifierFlags]) #define EV_UDMODIFIERS(e) \ @@ -6130,15 +6144,6 @@ flag set (this is probably a bug in the OS). code = fnKeysym; } - /* are there modifiers? */ - emacs_event->modifiers = 0; - - if (flags & NSEventModifierFlagHelp) - emacs_event->modifiers |= hyper_modifier; - - if (flags & NSEventModifierFlagShift) - emacs_event->modifiers |= shift_modifier; - /* The ⌘ and ⌥ modifiers can be either shift-like (for alternate character input) or control-like (as command prefix). If we have only shift-like modifiers, then we should use the @@ -6155,8 +6160,14 @@ untranslated characters (returned by the modifier keys, which returns 0 for shift-like modifiers. Therefore its return value is the set of control-like modifiers. */ - unsigned int control_modifiers = EV_MODIFIERS2 (flags); - emacs_event->modifiers |= control_modifiers; + emacs_event->modifiers = EV_MODIFIERS2 (flags); + + /* Function keys (such as the F-keys, arrow keys, etc.) behave + as though the fn key has been pressed even when it hasn't. + We need to unset the fn modifier if one of these keys is + pressed. FIXME: Avoid setting it in the first place. */ + if (fnKeysym && (flags & NS_FUNCTION_KEY_MASK)) + emacs_event->modifiers ^= parse_solitary_modifier (ns_function_modifier); if (NS_KEYLOG) fprintf (stderr, "keyDown: code =%x\tfnKey =%x\tflags = %x\tmods = %x\n", -- 2.16.1 --mP3DRpeJDSE+ciuQ--