Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blocking alt keys on macOS #768

Closed
chrisduerr opened this issue Jan 21, 2019 · 17 comments
Closed

Blocking alt keys on macOS #768

chrisduerr opened this issue Jan 21, 2019 · 17 comments
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - macos

Comments

@chrisduerr
Copy link
Contributor

chrisduerr commented Jan 21, 2019

In a terminal emulator like Alacritty, it's possible to send control characters directly to the shell using the alt and control keys. However this gets problematic with keyboard layouts that use the alt key for accessing alternative key characters. This is mainly an issue on macOS since other platforms usually use the AltGr key.

The concrete problem is that when alt is held down, the modified key is received, instead of the normal key. So for example Option + F will generate ƒ instead of f. In iTerm it's possible to pick between sending escapes and sending modified keys, however Alacritty currently can't offer that since winit will always send the modified keys.

Now I'm not really familiar with how this works under the hood in winit, but ideally I'd like an option in the window builder (maybe possible to change at runtime?) to prevent alt from modifying the characters, so that Option + F will still send f.

Alternatively it would be necessary for Alacritty to define its own mappings and ignore the ReceivedCharacter event, which would likely result in a bit of duplication between winit and Alacritty.

More information about this issue can be found here:
alacritty/alacritty#2017

@eraserhd
Copy link

Also alacritty/alacritty#1610

@kjmph
Copy link

kjmph commented Feb 4, 2020

I caught this sight of this bug from Alacritty. See my misguided post from alacritty/alacritty#2017. I opened a pull request #1436. However, that is meant to be an example pull request, as I'm not a maintainer of winit, I'm not as familiar with how options work.

@goddessfreya goddessfreya added DS - macos C - needs investigation Issue must be confirmed and researched B - bug Dang, that shouldn't have happened labels Feb 4, 2020
@kjmph
Copy link

kjmph commented Feb 10, 2020

If anyone would like to test, the pull request went through a few rounds. I have an open pull request that fixes this issue:

#1449

If anyone can test to provide support for the pull request, there is a single change to alacritty:

diff --git a/alacritty/src/window.rs b/alacritty/src/window.rs
index 71327ba..7bc555a 100644
--- a/alacritty/src/window.rs
+++ b/alacritty/src/window.rs
@@ -265,7 +265,8 @@ impl Window {
             .with_title(title)
             .with_visible(false)
             .with_transparent(true)
-            .with_maximized(window_config.startup_mode() == StartupMode::Maximized);
+            .with_maximized(window_config.startup_mode() == StartupMode::Maximized)
+            .with_ignore_alt_modifier(true);

         match window_config.decorations {
             Decorations::Full => window,

@kchibisov
Copy link
Member

If someone is going to test it on alacritty they could apply the following patch to use mentioned PR in addition to the patch above.

diff --git a/Cargo.toml b/Cargo.toml
index 415ceb9..e0db73b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,3 +13,4 @@ incremental = false

 [patch.crates-io]
 servo-freetype-sys = { path = "servo-freetype-proxy" }
+winit = { git = "https://github.com/kjmph/winit", branch = "fix-alt-modifier" }

@jkp
Copy link

jkp commented Mar 30, 2020

If someone is going to test it on alacritty they could apply the following patch to use mentioned PR in addition to the patch above.

diff --git a/Cargo.toml b/Cargo.toml
index 415ceb9..e0db73b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,3 +13,4 @@ incremental = false

 [patch.crates-io]
 servo-freetype-sys = { path = "servo-freetype-proxy" }
+winit = { git = "https://github.com/kjmph/winit", branch = "fix-alt-modifier" }

What is this suppose to do? I built and tested and hoped it would allow me to map left-alt to escape as in terminal.app but that doesn't seem to be the case...what am I missing?

@kjmph
Copy link

kjmph commented Mar 30, 2020

Can you elaborate on your difficulty? I believe what is wrong is that you are attempting to build an un-modified Alacritty against this branch of Winit. However, this patch requires modifications to Alacritty after it went through review. It now defaults the window setting to false and Alacritty must enable it. I haven't opened that pull request for Alacritty, since the upstream Winit pull request needs to be merged first. Would you like me to open it so you can see if the changes work for you?

To clarify a bit as well, this patch treats Option as Alt. So, either Left-Option or Right-Option will result in the Alt modifier set on the raw key press event. Alacritty interprets Alt as Escape, and will do as you are suggesting, which is to send an escape code like Terminal.app. Without this patch, the diacritical mark will still be sent, resulting in oddities for the next key press event. See this bug report for more information:

alacritty/alacritty#2017

@jkp
Copy link

jkp commented Mar 30, 2020

Sorry, I should have been more specific. I applied the two patches to alacritty to build with your branch and expected the behaviour of alacritty to change such that I could use option as a meta key...I have no idea if the intent is that should work or not. There are a lot of threads about this and I got lost along the way :)

@kjmph
Copy link

kjmph commented Mar 30, 2020

There are a lot of threads! I agree it is hard to follow, I hope we can clear this up soon. If you copied the patches in these threads, they are outdated after the Winit team reviewed pull request. Here are the current patches required for Alacritty to support: (and my apologies that it isn't in a pull request yet.)

diff --git a/Cargo.toml b/Cargo.toml
index 415ceb9..6bd5c66 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,3 +13,4 @@ incremental = false
 
 [patch.crates-io]
 servo-freetype-sys = { path = "servo-freetype-proxy" }
+{ git = "https://github.com/kjmph/winit", branch = "fix-alt-modifier" }
diff --git a/alacritty.yml b/alacritty.yml
index e552166..63c11b5 100644
--- a/alacritty.yml
+++ b/alacritty.yml
@@ -347,6 +347,9 @@
 # Send ESC (\x1b) before characters when alt is pressed.
 #alt_send_esc: true
 
+# (macOS Only) When Option is pressed treat as alt modifier. Don't send diacritical marks.
+#option_as_alt: true
+
 #debug:
   # Display the time it takes to redraw each frame.
   #render_timer: false
diff --git a/alacritty/src/event.rs b/alacritty/src/event.rs
index cd4f82a..b22e3f1 100644
--- a/alacritty/src/event.rs
+++ b/alacritty/src/event.rs
@@ -504,6 +504,10 @@ impl<N: Notify + OnResize> Processor<N> {
                             processor.ctx.display_update_pending.font = Some(font);
                         }
 
+                        if cfg!(target_os = "macos") && processor.ctx.config.option_as_alt() != config.option_as_alt() {
+                            processor.ctx.window.set_option_as_alt(config.option_as_alt());
+                        }
+
                         *processor.ctx.config = config;
 
                         processor.ctx.terminal.dirty = true;
diff --git a/alacritty/src/window.rs b/alacritty/src/window.rs
index 71327ba..6cd9041 100644
--- a/alacritty/src/window.rs
+++ b/alacritty/src/window.rs
@@ -151,6 +151,11 @@ impl Window {
         // Set OpenGL symbol loader. This call MUST be after window.make_current on windows.
         gl::load_with(|symbol| windowed_context.get_proc_address(symbol) as *const _);
 
+        // get_platform_window could be invoked with Config instead of
+        // WindowConfig if this call is misplaced.
+        #[cfg(target_os = "macos")]
+        windowed_context.window().set_option_as_alt(config.option_as_alt());
+
         // On X11, embed the window inside another if the parent ID has been set
         #[cfg(not(any(target_os = "macos", windows)))]
         {
@@ -356,6 +361,11 @@ impl Window {
         self.window().set_simple_fullscreen(simple_fullscreen);
     }
 
+    #[cfg(target_os = "macos")]
+    pub fn set_option_as_alt(&mut self, option_as_alt: bool) {
+        self.window().set_option_as_alt(option_as_alt);
+    }
+
     #[cfg(not(any(target_os = "macos", target_os = "windows")))]
     pub fn wayland_display(&self) -> Option<*mut c_void> {
         self.window().wayland_display()
diff --git a/alacritty_terminal/src/config/mod.rs b/alacritty_terminal/src/config/mod.rs
index fd049af..a9dabc4 100644
--- a/alacritty_terminal/src/config/mod.rs
+++ b/alacritty_terminal/src/config/mod.rs
@@ -117,6 +117,10 @@ pub struct Config<T> {
     #[serde(default, deserialize_with = "failure_default")]
     alt_send_esc: DefaultTrueBool,
 
+    /// Option on macOS is treated as Alt modifier
+    #[serde(default, deserialize_with = "failure_default")]
+    option_as_alt: DefaultTrueBool,
+
     /// Shell startup directory
     #[serde(default, deserialize_with = "option_explicit_none")]
     pub working_directory: Option<PathBuf>,
@@ -197,6 +201,12 @@ impl<T> Config<T> {
         self.alt_send_esc.0
     }
 
+    /// Option on macOS is treated as Alt modifier
+    #[inline]
+    pub fn option_as_alt(&self) -> bool {
+        self.option_as_alt.0
+    }
+
     /// Keep the log file after quitting Alacritty
     #[inline]
     pub fn persistent_logging(&self) -> bool {

@kjmph
Copy link

kjmph commented Mar 30, 2020

If the patch fails, it is because I haven't merged Alacritty against master in a while. Again, waiting on Winit's merge first.

@Porkepix
Copy link

Can you elaborate on your difficulty? I believe what is wrong is that you are attempting to build an un-modified Alacritty against this branch of Winit. However, this patch requires modifications to Alacritty after it went through review. It now defaults the window setting to false and Alacritty must enable it. I haven't opened that pull request for Alacritty, since the upstream Winit pull request needs to be merged first. Would you like me to open it so you can see if the changes work for you?

To clarify a bit as well, this patch treats Option as Alt. So, either Left-Option or Right-Option will result in the Alt modifier set on the raw key press event. Alacritty interprets Alt as Escape, and will do as you are suggesting, which is to send an escape code like Terminal.app. Without this patch, the diacritical mark will still be sent, resulting in oddities for the next key press event. See this bug report for more information:

alacritty/alacritty#2017

Does that mean it will breaks all special characters or dead keys from the layout that require Alt to be typed? And that one will have to choose between having a functional Alt key, or a complete layout? :/

@kjmph
Copy link

kjmph commented Mar 30, 2020

Hello @Porkepix, can you clarify? By special characters, you mean to say you would like diacritical marks, yes? What do you mean by dead keys? Think of this as the parameter "Use Option as Meta key" in Terminal.app. If option_as_alt is configured false, as well as alt_send_esc then diacritical marks can still be inputted. This can still be turned off and on, like Terminal.app.

@kjmph
Copy link

kjmph commented Mar 30, 2020

Wait, I think what you are worried about is you want Left-Option to exhibit one behavior, and Right-Option to exhibit another?

@Porkepix
Copy link

Hello @Porkepix, can you clarify? By special characters, you mean to say you would like diacritical marks, yes? What do you mean by dead keys? Think of this as the parameter "Use Option as Meta key" in Terminal.app. If option_as_alt is configured false, as well as alt_send_esc then diacritical marks can still be inputted. This can still be turned off and on, like Terminal.app.

Well, so I just tested, and yes it breaks a complete layout of the keyboard :(, for example it become impossible to type ~, æ, œ, «» and many other characters.

Wait, I think what you are worried about is you want Left-Option to exhibit one behavior, and Right-Option to exhibit another?

I guess that's a solution?

@kjmph
Copy link

kjmph commented Mar 30, 2020

Ah, yes, that would be the goal of this configuration. The same thing is true using Terminal.app and iTerm2 with these settings. What terminal emulator do you use on Mac?

@Porkepix
Copy link

Ah, yes, that would be the goal of this configuration. The same thing is true using Terminal.app and iTerm2 with these settings. What terminal emulator do you use on Mac?

Well, iTerm2 mostly, but to give some context I'm more of a Linux user where Alacritty is one of my terminals and I kinda wanted to also use it on the imposed macbook I got from work, and that's where I discovered the issue and therefore gave up on using Alacritty on mac while still using it on my ArchLinux.
I get the feeling it's one more project that can in theory work on mac, build and run correctly, but fails due to what Apple decided to do differently from other OSes, especially *nix ones. So yeah, one can tinker and remap most of the things, but that's not the same as having it working completely and directly out of the box.

@kjmph
Copy link

kjmph commented Mar 31, 2020

Once this issue is resolved, I think we will find that most users find this an acceptable alternative given the ecosystem, as you point out. There are only two other oddities which need to be fixed after this, and Alacritty can feel like an integrated terminal emulator on a Mac.

@kchibisov
Copy link
Member

Done with keyboard v2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - macos
Development

No branches or pull requests

8 participants