From: Alan Third <alan@idiocy.org>
To: Ben McGinnes <ben@adversary.org>
Cc: Philipp Stephani <p.stephani2@gmail.com>,
Emacs developers <emacs-devel@gnu.org>,
Noam Postavsky <npostavs@users.sourceforge.net>
Subject: Re: Emacs 26 MacOS bugs
Date: Wed, 7 Feb 2018 22:55:05 +0000 [thread overview]
Message-ID: <20180207225505.GB1066@breton.holly.idiocy.org> (raw)
In-Reply-To: <20180207021301.zpvpexdkewztqxo4@adversary.org>
[-- Attachment #1: Type: text/plain, Size: 2073 bytes --]
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
[-- Attachment #2: 0001-Fix-modifier-keys-on-NS-port.patch --]
[-- Type: text/plain, Size: 6472 bytes --]
From b089831704c91fbd0bf0baa45ba6f0c78cf06e41 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
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
next prev parent reply other threads:[~2018-02-07 22:55 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-06 14:49 Emacs 26 MacOS bugs Richard Stallman
2018-02-06 17:30 ` Noam Postavsky
2018-02-06 23:28 ` Paul Eggert
2018-02-07 3:37 ` Eli Zaretskii
2018-02-10 14:55 ` Uwe Brauer
2018-02-10 15:18 ` Noam Postavsky
2018-02-07 3:52 ` Eli Zaretskii
2018-02-06 19:05 ` Ben McGinnes
2018-02-06 20:30 ` Noam Postavsky
2018-02-06 22:27 ` Ben McGinnes
2018-02-06 22:30 ` Ben McGinnes
2018-02-06 22:50 ` Noam Postavsky
2018-02-07 0:43 ` Ben McGinnes
2018-02-07 1:33 ` Noam Postavsky
2018-02-07 2:13 ` Ben McGinnes
2018-02-07 22:55 ` Alan Third [this message]
2018-02-09 16:35 ` Ben McGinnes
2018-02-09 16:57 ` Noam Postavsky
2018-02-09 17:22 ` Ben McGinnes
2018-02-09 18:13 ` Noam Postavsky
2018-02-10 1:46 ` Ben McGinnes
2018-02-10 1:47 ` Alan Third
2018-02-11 13:53 ` Philipp Stephani
2018-02-11 21:06 ` Alan Third
2018-02-07 20:01 ` Alan Third
2018-02-10 15:01 ` Alan Third
2018-02-10 22:45 ` Alan Third
2018-02-11 11:47 ` Alan Third
2018-02-11 15:52 ` Eli Zaretskii
2018-02-11 20:57 ` Alan Third
2018-02-13 19:41 ` Ben McGinnes
2018-02-13 19:46 ` Noam Postavsky
2018-02-11 20:43 ` Richard Stallman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180207225505.GB1066@breton.holly.idiocy.org \
--to=alan@idiocy.org \
--cc=ben@adversary.org \
--cc=emacs-devel@gnu.org \
--cc=npostavs@users.sourceforge.net \
--cc=p.stephani2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.