Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ For live updates on Fresh, [follow me on X](https://x.com/TheNoamLewis).
* **Highlights taller than the window are drawn again.**
* **Input**
* **Shift works with "Keyboard Report All Keys As Escape Codes"** - `Shift+A` typed `a` (#2880, reported by @akarinotomoshibi).
* **`Ctrl+/` works outside kitty again** (#2933) - toggle-comment fired under kitty but nowhere else, because the `0x1F` byte every other terminal sends for the chord was read as `Ctrl+_`. It also reaches a terminal panel's child process as `0x1F` now, instead of a literal `/`.
* **Pasting works in the New Workspace and Run Agent dialogs**, in daemon mode too.
* **Settings & commands**
* **Settings toggles actually stick**
Expand Down
56 changes: 56 additions & 0 deletions crates/fresh-editor/src/app/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,62 @@ impl Editor {
.expect("editor base layer always owns the keyboard")
}

/// Handle a key press that a terminal reported, resolving which of its two
/// readings the keymap should see.
///
/// A chord is both a physical key plus modifiers (`Ctrl+Shift+7`) and the
/// character that key types (`&` on a US layout, `/` on a German one). The
/// parser reports both when they disagree — see
/// [`fresh_input_parser::KeyPress`] — because neither is right on its own:
/// binding the physical chord leaves a German user's `Ctrl+/` firing
/// `set_bookmark` (sinelaw/fresh#2933), and binding the typed character
/// breaks every US `Ctrl+Shift+<digit>`.
///
/// **The keymap decides.** The layout reading is tried first and used only
/// if something is actually bound to it; otherwise the physical chord is
/// handled exactly as before. So a US layout is unaffected — nothing binds
/// `ctrl+&`, so `Ctrl+Shift+7` still reaches `set_bookmark` — while a German
/// layout resolves the same keystroke to `ctrl+/`.
///
/// One chord can only mean one thing, so this is a precedence, not a
/// merge: where a keymap binds both readings, the layout one wins and the
/// physical chord is unreachable from that key. A user who wants the other
/// way round rebinds it.
pub fn handle_key_press(&mut self, press: fresh_input_parser::KeyPress) -> AnyhowResult<()> {
let (code, modifiers) = self
.layout_reading(&press)
.unwrap_or((press.code, press.modifiers));
self.handle_key(code, modifiers)
}

/// The chord built from what the key types on this layout, if the keymap
/// binds it. `None` when there is no distinct layout character, or when
/// nothing is bound to it and the physical chord should be used instead.
fn layout_reading(
&self,
press: &fresh_input_parser::KeyPress,
) -> Option<(crossterm::event::KeyCode, crossterm::event::KeyModifiers)> {
use crate::input::keybindings::Action;
use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};

let layout_char = press.layout_char?;
// Shift is spent producing the character, so it is not part of the
// chord built from it: the German `/` is `ctrl+/`, not `ctrl+shift+/`.
let modifiers = press.modifiers - KeyModifiers::SHIFT;
let code = KeyCode::Char(layout_char);
let action = self
.keybindings
.read()
.ok()?
.resolve(&KeyEvent::new(code, modifiers), self.get_key_context());
// `InsertChar` is the resolver's "nothing bound, just type it" answer.
// Typing is the physical key's job, not this reading's.
match action {
Action::None | Action::InsertChar(_) => None,
_ => Some((code, modifiers)),
}
}

/// Handle a key event and return whether it was handled
/// This is the central key handling logic used by both main.rs and tests
pub fn handle_key(
Expand Down
17 changes: 10 additions & 7 deletions crates/fresh-editor/src/app/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,18 +303,21 @@ impl Editor {
/// disturbing the live cursor.
///
/// Returns whether the editor wants the next frame redrawn.
pub fn handle_input_event(&mut self, event: crossterm::event::Event) -> anyhow::Result<bool> {
pub fn handle_input_event(&mut self, event: fresh_input_parser::Event) -> anyhow::Result<bool> {
use crate::input::is_keystroke;
use crossterm::event::Event as Ev;
use fresh_input_parser::{Event as Ev, KeyPress};

match event {
Ev::Key(key_event) if is_keystroke(key_event.kind) => {
let key_code = format!("{:?}", key_event.code);
let modifiers = format!("{:?}", key_event.modifiers);
Ev::Key(press) if is_keystroke(press.kind) => {
let key_code = format!("{:?}", press.code);
let modifiers = format!("{:?}", press.modifiers);
self.active_window_mut()
.log_keystroke(&key_code, &modifiers);
let translated = self.key_translator().translate(key_event);
self.handle_key(translated.code, translated.modifiers)?;
// The calibration translator rewrites the physical chord only;
// the layout character rides along untouched, since it says
// what the key types rather than which chord arrived.
let translated = self.key_translator().translate(press.event);
self.handle_key_press(KeyPress::with_layout_char(translated, press.layout_char))?;
// If `paste()` just took the async placeholder path,
// skip the otherwise-automatic render for this
// keystroke. The placeholder is sitting in the
Expand Down
4 changes: 2 additions & 2 deletions crates/fresh-editor/src/app/toggle_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,9 +434,9 @@ impl Editor {
/// normal input handling for it and re-render. This lives on `Editor` so
/// every event loop (the local terminal loop in `main.rs` and the daemon
/// server loop) shares one dismissal path rather than duplicating it.
pub fn maybe_dismiss_wave_animation(&mut self, event: &crossterm::event::Event) -> bool {
pub fn maybe_dismiss_wave_animation(&mut self, event: &fresh_input_parser::Event) -> bool {
use crate::input::is_keystroke;
use crossterm::event::Event;
use fresh_input_parser::Event;
if !self.wave_animation_active() {
return false;
}
Expand Down
37 changes: 18 additions & 19 deletions crates/fresh-editor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use windows_sys::Win32::System::Console::{AttachConsole, ATTACH_PARENT_PROCESS};

use anyhow::{Context, Result as AnyhowResult};
use clap::{CommandFactory, FromArgMatches, Parser};
use crossterm::event::{poll as event_poll, read as event_read, Event as CrosstermEvent};
use crossterm::event::{poll as event_poll, read as event_read};
use fresh::input::key_translator::KeyTranslator;
#[cfg(target_os = "linux")]
use fresh::services::gpm::{gpm_to_crossterm, GpmClient};
Expand All @@ -18,6 +18,7 @@ use fresh::{
services::release_checker, services::remote, services::signal_handler,
services::tracing_setup::TracingHandles, workspace,
};
use fresh_input_parser::Event as InputEvent;
use ratatui::Terminal;
use std::{
io::{self, stdout},
Expand Down Expand Up @@ -5752,7 +5753,7 @@ fn run_event_loop(
let reader = VtInputReader::spawn();

let mut input_parser = InputParser::new();
let mut event_buffer: std::collections::VecDeque<CrosstermEvent> =
let mut event_buffer: std::collections::VecDeque<InputEvent> =
std::collections::VecDeque::new();

let result = run_event_loop_common(
Expand All @@ -5761,7 +5762,7 @@ fn run_event_loop(
workspace_enabled,
key_translator,
terminal_modes,
|timeout| -> AnyhowResult<Option<CrosstermEvent>> {
|timeout| -> AnyhowResult<Option<InputEvent>> {
// Return buffered events first
if let Some(event) = event_buffer.pop_front() {
return Ok(Some(event));
Expand All @@ -5787,16 +5788,16 @@ fn run_event_loop(
}
Some(VtInputEvent::Resize) => {
if let Ok((cols, rows)) = crossterm::terminal::size() {
event_buffer.push_back(CrosstermEvent::Resize(cols, rows));
event_buffer.push_back(InputEvent::Resize(cols, rows));
}
got_any = true;
}
Some(VtInputEvent::FocusGained) => {
event_buffer.push_back(CrosstermEvent::FocusGained);
event_buffer.push_back(InputEvent::FocusGained);
got_any = true;
}
Some(VtInputEvent::FocusLost) => {
event_buffer.push_back(CrosstermEvent::FocusLost);
event_buffer.push_back(InputEvent::FocusLost);
got_any = true;
}
None => break,
Expand Down Expand Up @@ -5860,7 +5861,7 @@ fn run_event_loop(
/// consecutive recovered panics and, past a small threshold, surface an error
/// so the caller shuts the loop down cleanly instead of busy-looping. Any
/// successful read resets the counter.
fn safe_event_read() -> std::io::Result<Option<CrosstermEvent>> {
fn safe_event_read() -> std::io::Result<Option<InputEvent>> {
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::sync::atomic::{AtomicU32, Ordering};

Expand All @@ -5870,7 +5871,7 @@ fn safe_event_read() -> std::io::Result<Option<CrosstermEvent>> {
match catch_unwind(AssertUnwindSafe(event_read)) {
Ok(res) => {
CONSECUTIVE_PANICS.store(0, Ordering::Relaxed);
res.map(Some)
res.map(|event| Some(InputEvent::from(event)))
}
Err(_) => {
let n = CONSECUTIVE_PANICS.fetch_add(1, Ordering::Relaxed) + 1;
Expand Down Expand Up @@ -5899,14 +5900,14 @@ fn run_event_loop_common<F>(
mut poll_event: F,
) -> AnyhowResult<()>
where
F: FnMut(Duration) -> AnyhowResult<Option<CrosstermEvent>>,
F: FnMut(Duration) -> AnyhowResult<Option<InputEvent>>,
{
use std::time::Instant;

const FRAME_DURATION: Duration = Duration::from_millis(16); // 60fps
let mut last_render = Instant::now();
let mut needs_render = true;
let mut pending_event: Option<CrosstermEvent> = None;
let mut pending_event: Option<InputEvent> = None;
// Time of the last real input event, used to start the wave-animation
// screensaver after the configured idle period. Read from the editor's
// injected time source so tests can drive idle time deterministically.
Expand Down Expand Up @@ -6062,7 +6063,7 @@ where
// Event debug dialog receives ALL RAW events (before any translation or processing)
// This is essential for diagnosing terminal keybinding issues
if editor.active_window().is_event_debug_active() {
if let CrosstermEvent::Key(key_event) = event {
if let InputEvent::Key(key_event) = &event {
if fresh::input::is_keystroke(key_event.kind) {
editor
.active_window_mut()
Expand All @@ -6080,7 +6081,7 @@ where
// tracing spans + key-translation that depend on
// event-loop-local state.
let _span = match &event {
CrosstermEvent::Key(key_event) if fresh::input::is_keystroke(key_event.kind) => Some(
InputEvent::Key(key_event) if fresh::input::is_keystroke(key_event.kind) => Some(
tracing::trace_span!(
"handle_key",
code = ?key_event.code,
Expand All @@ -6104,7 +6105,7 @@ fn poll_with_gpm(
reader: &mut fresh::services::tty_input::TtyReader,
gpm_client: Option<&GpmClient>,
timeout: Duration,
) -> AnyhowResult<Option<CrosstermEvent>> {
) -> AnyhowResult<Option<InputEvent>> {
use nix::poll::{poll, PollFd, PollFlags, PollTimeout};
use std::os::unix::io::{AsRawFd, BorrowedFd};

Expand Down Expand Up @@ -6158,7 +6159,7 @@ fn poll_with_gpm(
match gpm.read_event() {
Ok(Some(gpm_event)) => {
if let Some(mouse_event) = gpm_to_crossterm(&gpm_event) {
return Ok(Some(CrosstermEvent::Mouse(mouse_event)));
return Ok(Some(InputEvent::Mouse(mouse_event)));
} else {
tracing::debug!("GPM event could not be converted to crossterm event");
}
Expand All @@ -6183,13 +6184,11 @@ fn poll_with_gpm(

/// Skip stale mouse move events, return the latest one.
/// If we read a non-move event while draining, return it as pending.
fn coalesce_mouse_moves(
event: CrosstermEvent,
) -> AnyhowResult<(CrosstermEvent, Option<CrosstermEvent>)> {
fn coalesce_mouse_moves(event: InputEvent) -> AnyhowResult<(InputEvent, Option<InputEvent>)> {
use crossterm::event::MouseEventKind;

// Only coalesce mouse moves
if !matches!(&event, CrosstermEvent::Mouse(m) if m.kind == MouseEventKind::Moved) {
if !matches!(&event, InputEvent::Mouse(m) if m.kind == MouseEventKind::Moved) {
return Ok((event, None));
}

Expand All @@ -6206,7 +6205,7 @@ fn coalesce_mouse_moves(
let Some(next) = safe_event_read()? else {
continue;
};
if matches!(&next, CrosstermEvent::Mouse(m) if m.kind == MouseEventKind::Moved) {
if matches!(&next, InputEvent::Mouse(m) if m.kind == MouseEventKind::Moved) {
latest = next; // Newer move, skip the old one
} else {
return Ok((latest, Some(next))); // Hit a click/key, save it
Expand Down
13 changes: 7 additions & 6 deletions crates/fresh-editor/src/server/editor_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::sync::mpsc;
use std::sync::Arc;
use std::time::{Duration, Instant};

use crossterm::event::Event;
use fresh_input_parser::Event;
use ratatui::Terminal;

use crate::app::Editor;
Expand Down Expand Up @@ -1533,10 +1533,10 @@ impl EditorServer {
}

match event {
Event::Key(key_event) => {
if crate::input::is_keystroke(key_event.kind) {
Event::Key(press) => {
if crate::input::is_keystroke(press.kind) {
editor
.handle_key(key_event.code, key_event.modifiers)
.handle_key_press(press)
.map_err(|e| io::Error::other(e.to_string()))?;
Ok(true)
} else {
Expand Down Expand Up @@ -1684,8 +1684,9 @@ mod wave_dismiss_tests {
use crate::config::Config;
use crate::config_io::DirectoryContext;
use crossterm::event::{
Event, KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind,
KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind,
};
use fresh_input_parser::Event;
use std::sync::Arc;
use std::time::Duration;

Expand Down Expand Up @@ -1853,7 +1854,7 @@ mod wave_dismiss_tests {
);

let consumed = server
.handle_event(Event::Key(KeyEvent::new(
.handle_event(Event::key(KeyEvent::new(
KeyCode::Char('a'),
KeyModifiers::empty(),
)))
Expand Down
4 changes: 1 addition & 3 deletions crates/fresh-editor/src/server/input_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@

use std::time::{Duration, Instant};

use crossterm::event::Event;

pub use fresh_input_parser::InputParser;
pub use fresh_input_parser::{Event, InputParser, KeyPress};

/// How long a lone `ESC` stays buffered before it is resolved as the Escape
/// key. Mirrors the tty reader's grace window: long enough that a control
Expand Down
2 changes: 1 addition & 1 deletion crates/fresh-editor/src/server/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::server::ipc::{ServerConnection, ServerListener, SocketPaths};
use crate::server::protocol::{
ClientControl, ServerControl, ServerHello, TermSize, VersionMismatch, PROTOCOL_VERSION,
};
use crossterm::event::Event;
use fresh_input_parser::Event;

/// Server configuration
#[derive(Debug, Clone)]
Expand Down
33 changes: 32 additions & 1 deletion crates/fresh-editor/src/services/terminal/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ fn control_byte(c: char) -> Option<u8> {
'\\' | '4' => 0x1c,
']' | '5' => 0x1d,
'^' | '6' => 0x1e,
'_' | '7' => 0x1f,
// Ctrl+/ is the same `US` byte as Ctrl+_ and Ctrl+7. Without the '/'
// arm the chord fell through to the plain-character path and the child
// saw a literal `/` — which is what a kitty-protocol terminal, where
// the chord arrives as Ctrl+/ rather than Ctrl+_, hit.
'_' | '7' | '/' => 0x1f,
'@' | '2' => 0x00, // NUL
' ' => 0x00, // Ctrl+Space = NUL
'?' => 0x7f, // DEL
Expand Down Expand Up @@ -373,6 +377,33 @@ mod tests {
assert_eq!(seq(KeyCode::Up, alt), "\x1b[1;3A");
}

/// Every spelling of the `US` chord has to reach the child as 0x1F. `/` was
/// missing from the table, so Ctrl+/ — the way the chord arrives from a
/// kitty-protocol terminal, and now from the legacy path too — fell through
/// to the plain-character branch and the child saw a bare `/`.
#[test]
fn ctrl_slash_reaches_the_child_as_us() {
let ctrl = KeyModifiers::CONTROL;

for key in ['/', '_', '7'] {
assert_eq!(
key_to_pty_bytes(KeyCode::Char(key), ctrl, false),
Some(vec![0x1f]),
"Ctrl+{key} should send 0x1F"
);
}

// Ctrl+Alt is deliberately not asserted: it is the one part of this
// encoding that varies by platform, since Windows reports AltGr as
// Ctrl+Alt and routes it to the plain-character path instead.

// Without Ctrl it is still an ordinary slash.
assert_eq!(
key_to_pty_bytes(KeyCode::Char('/'), KeyModifiers::empty(), false),
Some(vec![b'/'])
);
}

/// Home/End/PageUp/PageDown/Insert accepted only Ctrl (or nothing at all),
/// so every other modifier on them was silently dropped.
#[test]
Expand Down
Loading
Loading