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
5 changes: 1 addition & 4 deletions src/core/engine/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7723,10 +7723,7 @@ impl Engine {
}
}
} else if !text.is_empty() {
self.terminal_write(b"\x1b[200~");
self.terminal_write(text.as_bytes());
self.terminal_write(b"\x1b[201~");
self.poll_terminal();
self.terminal_paste(text);
}
return;
}
Expand Down
35 changes: 34 additions & 1 deletion src/core/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4080,6 +4080,39 @@ impl Engine {
/// registry, then either open the CLI-supplied path or restore the
/// previous session. Both TUI and GTK call this identically.
pub fn startup(&mut self, file_path: Option<&Path>) {
self.startup_inner(file_path, true);
}

/// [`Engine::startup`] minus the per-workspace session restore — the
/// constructor deterministic-geometry tests must use.
///
/// [`Engine::new_for_test`] is **not** sufficient on its own. It builds
/// the engine from in-memory `Settings::default()` / `SessionState::default()`,
/// but `startup(None)` then calls `restore_session_files()`, which performs
/// a *second, independent* disk read —
/// `SessionState::load_for_workspace(&self.cwd)` — keyed on the process's
/// `current_dir()` at construction time and entirely unrelated to whichever
/// `Settings`/`SessionState` the engine was built with.
/// `SessionState::save_for_workspace` is stubbed out under `cfg(test)`, but
/// `load_for_workspace` deliberately is not (several tests write a workspace
/// session file and assert it is restored). So on a machine that happens to
/// have a real `~/.config/vimcode/sessions/<hash>.json` for the checkout the
/// test binary runs in — entirely plausible for a self-hosting editor whose
/// developers edit it with itself — `startup(None)` reopens that session's
/// files and splits, `new_tab()` allocates a fresh `WindowId` per extra
/// file, and `windows.len()` stops being 1. Geometry measured against that
/// layout is then machine-dependent.
///
/// This entry point skips the restore entirely, so the resulting engine
/// depends on nothing but its in-memory defaults and the explicit
/// `file_path` argument.
pub fn startup_without_session_restore(&mut self, file_path: Option<&Path>) {
self.startup_inner(file_path, false);
}

/// Shared body of [`Engine::startup`] and
/// [`Engine::startup_without_session_restore`].
fn startup_inner(&mut self, file_path: Option<&Path>, restore_session: bool) {
self.plugin_init();
self.ext_refresh();
if let Some(path) = file_path {
Expand All @@ -4088,7 +4121,7 @@ impl Engine {
} else {
let _ = self.open_file_with_mode(path, OpenMode::Permanent);
}
} else {
} else if restore_session {
self.restore_session_files();
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/core/engine/terminal_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,22 @@ impl Engine {
}
}

/// Paste `text` into the active pane's PTY, then poll it so the echo
/// lands in the frame the caller is about to paint.
///
/// Delegates the bracketed-paste decision to quadraui's
/// `TerminalSession::paste` (quadraui#343/#415), which wraps in
/// `ESC[200~ … ESC[201~` only when the child has actually enabled DEC
/// private mode 2004. Both call sites used to hand-roll an
/// *unconditional* wrap, which leaked literal `[200~` bytes into programs
/// that do not strip them (`cat`, `less`, a shell without a line editor).
pub fn terminal_paste(&mut self, text: &str) {
if let Some(term) = self.active_terminal_mut() {
term.paste(text);
}
self.poll_terminal();
}

/// Resize all terminal panes (shared panel height).
pub fn terminal_resize(&mut self, cols: u16, rows: u16) {
for slot in &mut self.terminal_panes {
Expand Down
84 changes: 84 additions & 0 deletions src/core/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,90 @@ fn test_qa_bang_force_quits() {
assert_eq!(action, EngineAction::Quit);
}

/// `Engine::startup_without_session_restore` must ignore a per-workspace
/// session file that `Engine::startup` would have honoured.
///
/// This is the ambient-input class `TuiShellApp::new_for_test` exists to close:
/// `Engine::new_for_test()` only replaces the two *global* config reads
/// (`settings.json` / `session.json`), while `restore_session_files()` does a
/// second, independent `SessionState::load_for_workspace(&self.cwd)` read keyed
/// on the process's cwd. On a machine that has ever saved a session for the
/// checkout the test binary runs in, that read reopens real files and splits
/// and `windows.len()` stops being 1 — which is exactly what the TUI drag test's
/// setup-sanity assertion trips on.
///
/// The first half of this test is the control: it proves the fixture session
/// file on disk really is restorable, so a green second half means the restore
/// was *skipped*, not that the fixture was inert.
#[test]
fn test_startup_without_session_restore_ignores_workspace_session() {
use crate::core::session::SessionState;
let dir = std::env::temp_dir();
let p1 = dir.join("vimcode_no_restore_a.txt");
let p2 = dir.join("vimcode_no_restore_b.txt");
std::fs::write(&p1, "aaa").unwrap();
std::fs::write(&p2, "bbb").unwrap();

let workspace_dir = dir.join("vimcode_no_restore_test_ws");
std::fs::create_dir_all(&workspace_dir).unwrap();
let mut ws_session = SessionState::default();
ws_session.open_files = vec![p1.clone(), p2.clone()];
ws_session.active_file = Some(p2.clone());
// save_for_workspace is a no-op under #[cfg(test)], so write directly.
let session_path = SessionState::session_path_for_workspace(&workspace_dir);
if let Some(parent) = session_path.parent() {
std::fs::create_dir_all(parent).unwrap();
}
std::fs::write(
&session_path,
serde_json::to_string_pretty(&ws_session).unwrap(),
)
.unwrap();

// Control: the on-disk session really does restore two tabs.
let mut restoring = Engine::new_for_test();
restoring.cwd = workspace_dir.clone();
restoring.settings.swap_file = false;
restoring.restore_session_files();
assert_eq!(
restoring.active_group().tabs.len(),
2,
"control: the fixture workspace session must be restorable, otherwise \
the assertion below proves nothing"
);

// Subject: the deterministic startup path must not touch it at all.
let mut deterministic = Engine::new_for_test();
deterministic.cwd = workspace_dir.clone();
deterministic.settings.swap_file = false;
let tabs_before = deterministic.active_group().tabs.len();
let windows_before = deterministic.windows.len();
deterministic.startup_without_session_restore(None);
assert_eq!(
deterministic.active_group().tabs.len(),
tabs_before,
"startup_without_session_restore must not reopen the workspace session's files"
);
assert_eq!(
deterministic.windows.len(),
windows_before,
"startup_without_session_restore must not allocate windows for restored files"
);
assert!(
!deterministic
.buffer_manager
.iter()
.any(|(_, s)| s.file_path.as_deref() == Some(p1.as_path())
|| s.file_path.as_deref() == Some(p2.as_path())),
"no buffer should have been created for a session-listed file"
);

let _ = std::fs::remove_file(&p1);
let _ = std::fs::remove_file(&p2);
let _ = std::fs::remove_file(&session_path);
let _ = std::fs::remove_dir(&workspace_dir);
}

#[test]
fn test_restore_session_files_opens_separate_tabs() {
use crate::core::session::SessionState;
Expand Down
43 changes: 40 additions & 3 deletions src/gtk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1888,7 +1888,14 @@ impl App {
}

#[allow(clippy::too_many_lines)]
fn handle_key_press(&mut self, key_name: String, unicode: Option<char>, ctrl: bool, alt: bool) {
fn handle_key_press(
&mut self,
key_name: String,
unicode: Option<char>,
ctrl: bool,
shift: bool,
alt: bool,
) {
// ── Shared modal keyboard rung (#734 slice 1) ──────────────────
// `render::route_modal_key` states the spell-suggestion → dialog →
// context-menu ladder once for both backends. GTK used to hand-roll
Expand Down Expand Up @@ -1962,6 +1969,30 @@ impl App {
return;
}

// ── Shared terminal (PTY) keyboard rung (#758 / #734 slice 3) ──
// GTK had no terminal rung at all between #540 and this change: the
// block that forwarded keys to the PTY lived in the Relm4 `view!`'s
// `EventControllerKey` closure and was deleted with it, so every key
// typed into a focused terminal fell through to `Engine::handle_key`
// and ran vim commands on the *editor buffer* (#471). It sits here,
// directly below the focus owners and above the debug F-keys, so the
// ladder matches TUI's exactly: `route_focus_key` already returns
// `FocusKeyRoute::None` while the terminal holds focus, and a focused
// terminal must take F5/F9/F10/F11 to the PTY rather than the
// debugger — which is what `vim`/`htop` running inside it expect.
if render::route_terminal_key(
&mut self.engine.borrow_mut(),
&key_name,
unicode,
ctrl,
shift,
alt,
) {
self.sync_plus_register_to_clipboard();
self.draw_needed.set(true);
return;
}

// Debug F-keys must reach the engine regardless of which panel
// has focus — F5 (continue), F9 (breakpoint), F10 (step over),
// F11 (step in) are global debugger commands.
Expand Down Expand Up @@ -6875,13 +6906,19 @@ impl quadraui::ShellApp for App {
}
};
if !key_name.is_empty() || unicode.is_some() {
self.handle_key_press(key_name, unicode, modifiers.ctrl, modifiers.alt);
self.handle_key_press(
key_name,
unicode,
modifiers.ctrl,
modifiers.shift,
modifiers.alt,
);
}
}
UiEvent::CharTyped(c) => {
// Ctrl-modified characters arrive via KeyPressed; CharTyped is
// for IME-composed printable characters only.
self.handle_key_press(c.to_string(), Some(c), false, false);
self.handle_key_press(c.to_string(), Some(c), false, false, false);
}
UiEvent::Accelerator(id, _mods) => {
let id_str = id.as_str().to_string();
Expand Down
109 changes: 109 additions & 0 deletions src/gtk/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2762,6 +2762,115 @@ mod sidebar_panel_clicks {
);
}

// ── #758 / #734 slice 3: the shared terminal (PTY) keyboard rung ───────

/// GTK half of `terminal_ctrl_f_opens_the_painted_find_bar_via_shell_app`
/// (`tui_main/shell_app.rs`): with the terminal focused, Ctrl+F must open
/// the *terminal's* find bar, and subsequent characters must land in that
/// bar's query — through `App::handle` -> `handle_key_press` ->
/// `render::route_terminal_key` -> `Engine::handle_terminal_key`.
///
/// GTK had **no terminal keyboard rung at all** between #540 and #758:
/// the `if engine.borrow().terminal_has_focus { … }` block lived in the
/// Relm4 `view!`'s `EventControllerKey` closure and was deleted with it,
/// so Ctrl+F opened the editor's find/replace overlay and every other key
/// ran a vim command on the buffer while the user was looking at a shell
/// prompt (#471).
///
/// Asserts on rendered output (`CLAUDE.md` rule 1): the terminal
/// toolbar's painted `" FIND: …"` text (`render::build_terminal_toolbar`,
/// drawn by `draw_terminal_panel`), never `terminal_find_active`.
///
/// **Verified RED against unfixed `develop`:** without the
/// `render::route_terminal_key` call, Ctrl+F falls through to
/// `Engine::handle_key` and no `"FIND:"` ever paints — the second
/// assertion fires.
#[test]
fn terminal_ctrl_f_opens_the_painted_find_bar() {
let mut engine = Engine::new_for_test();
engine.settings.use_nerd_fonts = false;
// `terminal_new_tab` opens the panel and focuses it.
engine.terminal_new_tab(80, 10);

let mut h = harness(engine, 1400, 900);
h.driver.render();
assert!(
!h.driver.screen_contains("FIND:"),
"precondition: the terminal toolbar starts as a tab strip; painted: {:?}",
h.driver.painted_texts()
);

h.driver.ctrl_char('f');
h.driver.render();
assert!(
h.driver.screen_contains("FIND:"),
"Ctrl+F with the terminal focused must open the terminal find bar, \
not the editor find/replace overlay; painted: {:?}",
h.driver.painted_texts()
);

h.driver.type_char('z');
h.driver.render();
assert!(
h.driver.screen_contains("FIND: z"),
"characters typed after Ctrl+F must reach the terminal find query \
through the shared router; painted: {:?}",
h.driver.painted_texts()
);
}

/// A focused terminal must swallow ordinary keys so they never reach the
/// editor buffer. This is the user-visible shape of the missing GTK rung:
/// typing `x` at a shell prompt deleted a character from the *file*.
///
/// Asserts on the painted buffer text, with a positive control — the same
/// key on the same fixture with the terminal unfocused must delete the
/// character — so a fixture whose text could not change would fail.
///
/// **Verified RED against unfixed `develop`:** without the router call
/// the first `x` reaches `Engine::handle_key`, the painted line drops to
/// `QXWTERMGTK758` immediately, and the second assertion fires.
#[test]
fn focused_terminal_swallows_editor_keys_on_gtk() {
let build = |focused: bool| {
let mut engine = Engine::new_for_test();
engine.settings.use_nerd_fonts = false;
engine.buffer_mut().insert(0, "ZQXWTERMGTK758\n");
engine.terminal_new_tab(80, 6);
engine.terminal_has_focus = focused;
harness(engine, 1400, 900)
};

let mut h = build(true);
h.driver.render();
assert!(
h.driver.screen_contains("ZQXWTERMGTK758"),
"precondition: the buffer line must paint; painted: {:?}",
h.driver.painted_texts()
);

h.driver.type_char('x');
h.driver.render();
assert!(
h.driver.screen_contains("ZQXWTERMGTK758"),
"`x` with the terminal focused must go to the PTY, not delete a \
character from the editor buffer; painted: {:?}",
h.driver.painted_texts()
);

let mut control = build(false);
control.driver.render();
control.driver.type_char('x');
control.driver.render();
assert!(
control.driver.screen_contains("QXWTERMGTK758")
&& !control.driver.screen_contains("ZQXWTERMGTK758"),
"control: with the terminal unfocused `x` must delete the first \
character; painted: {:?}",
control.driver.painted_texts()
);
}

/// The sidebar hover rung must exist **on this backend at all**.
///
/// Before #754 the Source Control toolbar's hover highlight was driven by
Expand Down
Loading
Loading