fix(tui): normalize macOS SUPER (Cmd) to CONTROL for keyboard shortcuts (#2938) by idling11 · Pull Request #2943 · Hmbown/CodeWhale

idling11 · 2026-06-09T07:28:37Z

On macOS, terminal emulators may report Cmd (SUPER) instead of Ctrl (CONTROL) for keyboard shortcuts, depending on the terminal app and its configuration. This caused Ctrl+B, Ctrl+Alt+2, and other shortcuts to be inconsistent.

Fix:

Add normalize_macos_modifiers() in composer_ui.rs
On macOS: map SUPER to CONTROL when CONTROL is not already set
On other platforms: no-op
Apply normalization at the key event entry point in ui.rs

Tests:

normalize_macos_modifiers_maps_super_to_control
normalize_macos_modifiers_preserves_existing_control
normalize_macos_modifiers_leaves_alt_unchanged

Close #2938

Summary

Testing

cargo fmt --all -- --check
cargo clippy --workspace --all-targets --all-features
cargo test --workspace --all-features

Checklist

Updated docs or comments as needed
Added or updated tests where relevant
Verified TUI behavior manually if UI changes
Harvested/co-authored credit uses a GitHub numeric noreply address

…ts (Hmbown#2938) On macOS, terminal emulators may report Cmd (SUPER) instead of Ctrl (CONTROL) for keyboard shortcuts, depending on the terminal app and its configuration. This caused Ctrl+B, Ctrl+Alt+2, and other shortcuts to be inconsistent. Fix: - Add normalize_macos_modifiers() in composer_ui.rs - On macOS: map SUPER to CONTROL when CONTROL is not already set - On other platforms: no-op - Apply normalization at the key event entry point in ui.rs Tests: - normalize_macos_modifiers_maps_super_to_control - normalize_macos_modifiers_preserves_existing_control - normalize_macos_modifiers_leaves_alt_unchanged

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

gemini-code-assist

Code Review

This pull request introduces modifier normalization on macOS to map the SUPER (Cmd) key to CONTROL, ensuring keyboard shortcuts work consistently across different terminal emulators. The feedback highlights a critical issue where keeping the SUPER modifier alongside CONTROL would break exact modifier equality checks (e.g., key.modifiers == KeyModifiers::CONTROL). The reviewer suggests stripping SUPER entirely when mapping it to CONTROL and provides corresponding updates for both the normalization logic and the unit tests.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-09T07:29:34Z

+#[cfg(target_os = "macos")]
+pub(crate) fn normalize_macos_modifiers(modifiers: KeyModifiers) -> KeyModifiers {
+    if modifiers.contains(KeyModifiers::SUPER) && !modifiers.contains(KeyModifiers::CONTROL) {
+        modifiers | KeyModifiers::CONTROL
+    } else {
+        modifiers
+    }
+}


If SUPER is kept in the modifiers, exact modifier checks like key.modifiers == KeyModifiers::CONTROL (such as the Ctrl+S stashing shortcut in ui.rs) will fail because the modifiers will be evaluated as CONTROL | SUPER instead of just CONTROL. On macOS, we should strip SUPER entirely and ensure CONTROL is set whenever SUPER is present.

Suggested change

#[cfg(target_os = "macos")]

pub(crate) fn normalize_macos_modifiers(modifiers: KeyModifiers) -> KeyModifiers {

if modifiers.contains(KeyModifiers::SUPER) && !modifiers.contains(KeyModifiers::CONTROL) {

modifiers | KeyModifiers::CONTROL

} else {

modifiers

}

}

#[cfg(target_os = "macos")]

pub(crate) fn normalize_macos_modifiers(modifiers: KeyModifiers) -> KeyModifiers {

if modifiers.contains(KeyModifiers::SUPER) {

(modifiers & !KeyModifiers::SUPER) | KeyModifiers::CONTROL

} else {

modifiers

}

}

gemini-code-assist · 2026-06-09T07:29:34Z

+#[test]
+fn normalize_macos_modifiers_maps_super_to_control() {
+    use crate::tui::composer_ui::normalize_macos_modifiers;
+    // SUPER (Cmd) without CONTROL should gain CONTROL.
+    let normalized = normalize_macos_modifiers(KeyModifiers::SUPER);
+    assert!(normalized.contains(KeyModifiers::CONTROL));
+    assert!(normalized.contains(KeyModifiers::SUPER));
+}


Update the test to assert that SUPER is removed during normalization, ensuring exact modifier equality checks work correctly.

Suggested change

#[test]

fn normalize_macos_modifiers_maps_super_to_control() {

use crate::tui::composer_ui::normalize_macos_modifiers;

// SUPER (Cmd) without CONTROL should gain CONTROL.

let normalized = normalize_macos_modifiers(KeyModifiers::SUPER);

assert!(normalized.contains(KeyModifiers::CONTROL));

assert!(normalized.contains(KeyModifiers::SUPER));

}

#[test]

fn normalize_macos_modifiers_maps_super_to_control() {

use crate::tui::composer_ui::normalize_macos_modifiers;

// SUPER (Cmd) without CONTROL should gain CONTROL and lose SUPER.

let normalized = normalize_macos_modifiers(KeyModifiers::SUPER);

assert!(normalized.contains(KeyModifiers::CONTROL));

assert!(!normalized.contains(KeyModifiers::SUPER));

}

gemini-code-assist · 2026-06-09T07:29:34Z

+#[test]
+fn normalize_macos_modifiers_preserves_existing_control() {
+    use crate::tui::composer_ui::normalize_macos_modifiers;
+    // CONTROL already set — shouldn't be removed.
+    let normalized = normalize_macos_modifiers(KeyModifiers::CONTROL | KeyModifiers::SUPER);
+    assert!(normalized.contains(KeyModifiers::CONTROL));
+    assert!(normalized.contains(KeyModifiers::SUPER));
+}


Update the test to assert that SUPER is removed even when CONTROL is already present, ensuring exact modifier equality checks work correctly.

Suggested change

#[test]

fn normalize_macos_modifiers_preserves_existing_control() {

use crate::tui::composer_ui::normalize_macos_modifiers;

// CONTROL already set — shouldn't be removed.

let normalized = normalize_macos_modifiers(KeyModifiers::CONTROL | KeyModifiers::SUPER);

assert!(normalized.contains(KeyModifiers::CONTROL));

assert!(normalized.contains(KeyModifiers::SUPER));

}

#[test]

fn normalize_macos_modifiers_preserves_existing_control() {

use crate::tui::composer_ui::normalize_macos_modifiers;

// CONTROL already set — SUPER should be removed.

let normalized = normalize_macos_modifiers(KeyModifiers::CONTROL | KeyModifiers::SUPER);

assert!(normalized.contains(KeyModifiers::CONTROL));

assert!(!normalized.contains(KeyModifiers::SUPER));

}

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Hmbown · 2026-06-10T03:05:26Z

Thanks @idling11 — the normalization approach looks right, but Test (windows-latest) is failing and I believe it's the new tests rather than the logic: normalize_macos_modifiers_maps_super_to_control and normalize_macos_modifiers_preserves_existing_control assert the macOS mapping unconditionally, while the #[cfg(not(target_os = "macos"))] version is a no-op that returns SUPER unchanged — so both assertions fail off-macOS.

Could you either #[cfg(target_os = "macos")]-gate those two tests, or make the assertions platform-aware? One design question to consider while you're in there: with terminals that forward Cmd via the kitty/CSI-u protocol, a global Cmd→Ctrl remap turns e.g. Cmd+C into Ctrl+C (interrupt). If that's a real risk it may be worth scoping the remap to shortcuts we actually handle rather than the whole event stream. Happy to merge once Windows is green.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

greptile-apps Bot reviewed Jun 9, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jun 9, 2026

View reviewed changes

fix: strip SUPER from modifiers after normalization per review

1b5442f

greptile-apps Bot reviewed Jun 9, 2026

View reviewed changes

fix: gate macOS-specific modifier tests with #[cfg(target_os = "macos")]

7315ef3

greptile-apps Bot reviewed Jun 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tui): normalize macOS SUPER (Cmd) to CONTROL for keyboard shortcuts (#2938)#2943

fix(tui): normalize macOS SUPER (Cmd) to CONTROL for keyboard shortcuts (#2938)#2943
idling11 wants to merge 3 commits into
Hmbown:mainfrom
idling11:fix/macos-modifier-normalization

idling11 commented Jun 9, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

Hmbown commented Jun 10, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

idling11 commented Jun 9, 2026

Summary

Testing

Checklist

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Hmbown commented Jun 10, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants