diff --git a/CHANGELOG.md b/CHANGELOG.md index 3da0235d..3a7b3735 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A config file saved with a UTF-8 BOM no longer wipes your settings** — every + config-dir file is read by a loader that treats any parse error as "there is + no file", falling back to defaults. `serde_json` rejects the U+FEFF a BOM puts + before the opening brace, so a BOM didn't report a broken config — it reported + an absent one, and tty7 came up on defaults with nothing in the log to explain + it. Windows makes this easy to hit by accident: PowerShell's `>`, `Out-File` + and `Set-Content -Encoding utf8` all write a BOM, so editing `config.json` + from a shell was enough to lose every setting. `config.json`, `session.json` + (which lost every workspace the same way) and hand-authored `themes/*.yaml` + now skip a leading BOM. + - **New tabs and splits open in the right directory even when the shell can't be instrumented** ([#187](https://github.com/l0ng-ai/tty7/issues/187)) — a pane learned its directory from `OSC 7`, which only shells tty7 injects its diff --git a/src/core/config.rs b/src/core/config.rs index 01efaed6..32e2fe79 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -679,7 +679,7 @@ impl Config { // Missing/unreadable config is the common case — start with defaults. return Config::default(); }; - match serde_json::from_str::(&text) { + match serde_json::from_str::(strip_bom(&text)) { Ok(mut cfg) => { cfg.sanitize(); cfg @@ -824,6 +824,21 @@ pub fn config_path(file: &str) -> Option { Some(config_dir()?.join(file)) } +/// Drop a leading UTF-8 BOM so a hand-edited config still parses. +/// +/// `serde_json` rejects U+FEFF before the opening brace, and every config-dir +/// file is read by a loader that treats *any* parse error as "there is no +/// config" — so a BOM doesn't surface as an error, it silently resets the +/// user's settings. Windows makes that easy to hit by accident: PowerShell's +/// `>`, `Out-File` and `Set-Content -Encoding utf8` all write one, so a quick +/// `... | Set-Content config.json` is enough to lose every setting. +/// +/// `read_to_string` decodes the BOM to the single char U+FEFF, so this strips +/// the char rather than the three raw bytes. +pub fn strip_bom(text: &str) -> &str { + text.strip_prefix('\u{FEFF}').unwrap_or(text) +} + /// Write `bytes` to `path` atomically: write to a sibling temp file, fsync, then /// rename over the target. A crash/power-loss mid-write then leaves either the /// old file or the new one intact — never a truncated/half-written file that @@ -1417,8 +1432,19 @@ mod tests { set_config_dir(dir); } + /// Serialize the tests that write the shared `config.json`: they all resolve + /// the same pinned path, so without this they clobber each other's file. + static CONFIG_FILE: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + fn lock_config_file() -> std::sync::MutexGuard<'static, ()> { + // A poisoned lock only means another test failed mid-sequence; every + // holder rewrites the file from scratch, so the state is still sound. + CONFIG_FILE.lock().unwrap_or_else(|e| e.into_inner()) + } + #[test] fn save_load_and_shell_command_round_trip_through_disk() { + let _guard = lock_config_file(); pin_config_dir(); // Persist a config with a non-default shell + font + an SSH profile, then // read it back. @@ -1457,6 +1483,41 @@ mod tests { assert_eq!(args, vec!["-l".to_string()]); } + #[test] + fn a_utf8_bom_does_not_silently_reset_the_config() { + let _guard = lock_config_file(); + pin_config_dir(); + let path = Config::path().expect("pinned config dir"); + // Exactly what PowerShell's `>`, `Out-File` and `Set-Content -Encoding + // utf8` leave behind. + let text = "\u{FEFF}{\"font_size\": 21.0, \"restore_session\": false}"; + write_atomic(&path, text.as_bytes()).unwrap(); + + // The failure this guards is silent by construction: `load` turns *any* + // parse error into defaults, so a BOM didn't report a bad config — it + // reported no config, and the user's settings appeared to vanish. + let loaded = Config::load(); + assert_eq!(loaded.font_size, 21.0); + assert!(!loaded.restore_session); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn strip_bom_only_removes_a_leading_marker() { + assert_eq!(strip_bom("{}"), "{}"); + assert_eq!(strip_bom("\u{FEFF}{}"), "{}"); + // Only the first U+FEFF is a marker. A second one is content, and + // content that happens to be a BOM is still invalid JSON — stripping + // it too would be guessing at a file we can't rescue. + assert_eq!(strip_bom("\u{FEFF}\u{FEFF}{}"), "\u{FEFF}{}"); + // A BOM *inside* the document is data (U+FEFF is a legal string char), + // so it must survive untouched. + let inner = "{\"tab_title\":\"\u{FEFF}\"}"; + assert_eq!(strip_bom(inner), inner); + assert_eq!(strip_bom(""), ""); + } + #[test] fn ssh_profiles_default_empty_and_parse_from_json() { // Absent key → empty (a config predating profiles still loads). diff --git a/src/core/session.rs b/src/core/session.rs index 4e426489..ff971b35 100644 --- a/src/core/session.rs +++ b/src/core/session.rs @@ -300,7 +300,7 @@ impl Workspaces { /// `{active, tabs}` session, which migrates to a single open workspace so /// upgrading users keep their tabs (and their attached daemon panes). pub fn decode(text: &str) -> Result { - let value: serde_json::Value = serde_json::from_str(text)?; + let value: serde_json::Value = serde_json::from_str(crate::core::config::strip_bom(text))?; if value.get("workspaces").is_some() { return serde_json::from_value(value); } @@ -759,6 +759,25 @@ mod tests { )); } + #[test] + fn a_utf8_bom_does_not_discard_the_session() { + // `Session::load` treats a parse error as "no session", so a BOM on a + // hand-edited `session.json` doesn't warn — it drops every workspace + // and opens on the home page as if nothing had been saved. + // Legacy `{active, tabs}` shape, so this also covers the migration path. + let decoded = Workspaces::decode( + "\u{FEFF}{\"active\": 0, \"tabs\": [{\"pane\": {\"Leaf\": {\"cwd\": \"/work\"}}}]}", + ) + .expect("a BOM'd session still decodes"); + let tabs = &decoded + .workspaces + .first() + .expect("migrated workspace") + .session + .tabs; + assert_eq!(tabs.len(), 1); + } + #[test] fn session_defaults_fill_missing_fields() { // An empty object → default (active 0, no tabs). diff --git a/src/daemon/pane.rs b/src/daemon/pane.rs index 241fd3da..a7a167a2 100644 --- a/src/daemon/pane.rs +++ b/src/daemon/pane.rs @@ -1117,12 +1117,14 @@ impl DaemonPane { // really reading them. The proc query runs only when a // mark actually claims the prompt — about once per prompt. // See issue #26. - if signals.shell.as_ref().is_some_and(|s| s.at_prompt) - && foreground_running() - { - if let Some(s) = signals.shell.as_mut() { + if signals.shell.iter().any(|s| s.at_prompt) && foreground_running() { + for s in signals.shell.iter_mut() { s.at_prompt = false; } + // Clearing the flag can leave neighbours identical; + // they no longer describe a crossing, so don't spend + // a frame on each. + signals.shell.dedup(); } // SSH-context + coding-agent detection are process-table @@ -1822,7 +1824,9 @@ fn apply_signals(st: &mut PaneState, signals: SniffSignals) { st.cwd = Some(cwd); } } - if let Some(shell) = signals.shell { + // One frame per entry: the client needs every prompt-boundary crossing the + // chunk carried, not just where it ended up (see [`SniffSignals::shell`]). + for shell in signals.shell { // Windows: agent identity rides the C mark's command capture — ConPTY // has no foreground process group for the Unix 0.5 s poll to read an // argv from. `C;` detects, `D` cleared `command` so it applies @@ -2287,7 +2291,20 @@ struct ShellState { #[derive(Default)] struct SniffSignals { cwd: Option, - shell: Option, + /// Shell states completed in this chunk, in stream order — one entry per + /// `at_prompt` *transition*, not one per marker: a run of marks on the same + /// side of the prompt boundary folds into its latest state, so the ordinary + /// `D`/`A`/`B` chunk still yields the single entry it always did. + /// + /// Only the last state describes "what the shell is doing now", but the + /// client keys its prompt *cycle* off the false→true edge + /// (`terminal::remote::ShellState::cycle` — what releases a Tab handoff, see + /// `TerminalView::editor_handoff`). Collapsing to the last state alone hides + /// that edge whenever a whole command cycle (`C` … `D`) lands in one read — + /// routine over SSH, where a fast command's output arrives in a single + /// packet — and the handed-off prompt would then never come back to tty7's + /// line editor. + shell: Vec, /// Sentinel agent events completed in this chunk, in stream order — each /// one is a state-machine step, so unlike cwd/shell they must *all* apply /// (a `stop` directly after a `notification` still means "done"). @@ -2315,9 +2332,9 @@ impl OscSniffer { } } - /// Feed a chunk; return any cwd / shell-state change completed within it. (If a - /// chunk completes several markers, the last cwd / last shell state wins, which - /// is the only state worth reporting.) + /// Feed a chunk; return any cwd / shell-state change completed within it. (If + /// a chunk completes several cwd markers the last one wins; shell states keep + /// every prompt-boundary crossing — see [`SniffSignals::shell`].) fn feed(&mut self, bytes: &[u8]) -> SniffSignals { let mut signals = SniffSignals::default(); let shell = &mut self.shell; @@ -2326,7 +2343,15 @@ impl OscSniffer { signals.cwd = Some(path); } else if let Some(rest) = payload.strip_prefix(b"133;") { if handle_osc133(shell, rest) { - signals.shell = Some(shell.clone()); + match signals.shell.last_mut() { + // Still on the same side of the prompt boundary: fold in, + // latest wins (it carries the freshest exit code / command + // capture). Only a crossing earns its own entry. + Some(last) if last.at_prompt == shell.at_prompt => { + *last = shell.clone(); + } + _ => signals.shell.push(shell.clone()), + } } } else if let Some(event) = crate::core::cli_agent::parse_agent_event(payload) { signals.agent_events.push(event); @@ -3131,17 +3156,53 @@ mod tests { fn sniff_osc133_prompt() { let mut s = OscSniffer::new(); let b = s.feed(b"\x1b]133;B\x07"); - assert!(b.shell.as_ref().unwrap().active); - assert!(b.shell.as_ref().unwrap().at_prompt); + assert!(b.shell.last().unwrap().active); + assert!(b.shell.last().unwrap().at_prompt); let c = s.feed(b"\x1b]133;C\x07"); - assert!(!c.shell.as_ref().unwrap().at_prompt); + assert!(!c.shell.last().unwrap().at_prompt); // D (command finished) means no command is running, so we're back at the // prompt: at_prompt is true again (it also carries the exit code). let d = s.feed(b"\x1b]133;D;130\x07"); - assert!(d.shell.as_ref().unwrap().at_prompt); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, Some(130)); + assert!(d.shell.last().unwrap().at_prompt); + assert_eq!(d.shell.last().unwrap().last_exit_code, Some(130)); + } + + /// A whole command cycle inside ONE chunk still reports the prompt-boundary + /// crossing. The client counts `at_prompt` false→true edges to tell a fresh + /// prompt from a same-prompt redraw, and a Tab handoff only returns the line + /// to tty7's editor on that edge (`TerminalView::editor_handoff`). Reporting + /// just the chunk's final state hid the edge whenever `C` … `D` arrived + /// together — the norm over SSH, where a fast command's whole output lands in + /// one read — so one Tab in an ssh pane disabled the local editor for good. + #[test] + fn a_full_command_cycle_in_one_chunk_still_reports_leaving_the_prompt() { + let mut s = OscSniffer::new(); + s.feed(b"\x1b]133;A\x07\x1b]133;B\x07"); // sitting at the prompt + + // Enter → command → output → done → next prompt, all in one read. + let sig = + s.feed(b"\x1b]133;C;echo%20hi\x07hi\r\n\x1b]133;D;0\x07\x1b]133;A\x07\x1b]133;B\x07"); + let states: Vec = sig.shell.iter().map(|s| s.at_prompt).collect(); + assert_eq!( + states, + vec![false, true], + "the chunk must report leaving the prompt and coming back, not just the end state" + ); + assert_eq!(sig.shell.last().unwrap().last_exit_code, Some(0)); + assert_eq!(sig.shell.last().unwrap().command, None); + } + + /// The flip side: marks that stay on one side of the boundary fold into a + /// single state, so the ordinary prompt draw still costs exactly one frame. + #[test] + fn marks_on_the_same_side_of_the_prompt_boundary_fold_into_one_state() { + let mut s = OscSniffer::new(); + let sig = s.feed(b"\x1b]133;D;3\x07\x1b]133;A\x07\x1b]133;B\x07"); + assert_eq!(sig.shell.len(), 1, "D/A/B is one at-prompt state"); + assert!(sig.shell[0].at_prompt); + assert_eq!(sig.shell[0].last_exit_code, Some(3)); } /// The C mark's command capture (tty7 extension, PowerShell integration) — @@ -3156,7 +3217,7 @@ mod tests { // A submitted `claude --help` (space percent-encoded, as the // PowerShell body emits it). let c = s.feed(b"\x1b]133;C;claude%20--help\x07"); - let shell = c.shell.as_ref().unwrap(); + let shell = c.shell.last().unwrap(); assert!(!shell.at_prompt); assert_eq!(shell.command.as_deref(), Some("claude --help")); assert_eq!( @@ -3166,19 +3227,19 @@ mod tests { // The command finishing (D) clears the capture → the agent clears. let d = s.feed(b"\x1b]133;D;0\x07"); - let shell = d.shell.as_ref().unwrap(); + let shell = d.shell.last().unwrap(); assert_eq!(shell.command, None); assert_eq!(agent_from_shell_mark(shell, &custom), None); // A non-agent command sets the capture but detects nothing. let c = s.feed(b"\x1b]133;C;git%20status\x07"); - let shell = c.shell.as_ref().unwrap(); + let shell = c.shell.last().unwrap(); assert_eq!(shell.command.as_deref(), Some("git status")); assert_eq!(agent_from_shell_mark(shell, &custom), None); // A bare `C` (a foreign shell integration) leaves no capture. let c = s.feed(b"\x1b]133;C\x07"); - assert_eq!(c.shell.as_ref().unwrap().command, None); + assert_eq!(c.shell.last().unwrap().command, None); // A stray A/B mid-command (a nested/remote shell drawing its own // prompt) must NOT wipe the capture — only D (command finished) does. @@ -3186,21 +3247,21 @@ mod tests { // what keeps the agent chip alive while the agent runs. let _ = s.feed(b"\x1b]133;C;codex\x07"); let a = s.feed(b"\x1b]133;A\x1b]133;B\x07"); - assert_eq!(a.shell.as_ref().unwrap().command.as_deref(), Some("codex")); + assert_eq!(a.shell.last().unwrap().command.as_deref(), Some("codex")); let d = s.feed(b"\x1b]133;D;0\x07"); - assert_eq!(d.shell.as_ref().unwrap().command, None); + assert_eq!(d.shell.last().unwrap().command, None); // A multi-line command arrives %0A-joined (fish re-joins the split // list with it) and decodes back to real newlines. let c = s.feed(b"\x1b]133;C;echo%20a%0Aecho%20b\x07"); assert_eq!( - c.shell.as_ref().unwrap().command.as_deref(), + c.shell.last().unwrap().command.as_deref(), Some("echo a\necho b") ); // An all-whitespace payload is no capture, like a bare `C`. let c = s.feed(b"\x1b]133;C;%20%20\x07"); - assert_eq!(c.shell.as_ref().unwrap().command, None); + assert_eq!(c.shell.last().unwrap().command, None); } /// The Windows apply gate ([`shell_mark_capture_changed`]): detection @@ -3214,7 +3275,7 @@ mod tests { // A wrapper launch: capture set, but detection has no answer. let mut prev = ShellState::default(); - let c = s.feed(b"\x1b]133;C;.%5Cdev.ps1\x07").shell.unwrap(); + let c = s.feed(b"\x1b]133;C;.%5Cdev.ps1\x07").shell.pop().unwrap(); assert!(shell_mark_capture_changed(&prev, &c)); assert_eq!( agent_from_shell_mark(&c, &std::collections::HashMap::new()), @@ -3224,13 +3285,13 @@ mod tests { // Stray foreign prompt marks mid-command: same capture, no re-apply — // an agent branded by sentinel events keeps its chip. - let ab = s.feed(b"\x1b]133;A\x1b]133;B\x07").shell.unwrap(); + let ab = s.feed(b"\x1b]133;A\x1b]133;B\x07").shell.pop().unwrap(); assert!(!shell_mark_capture_changed(&prev, &ab)); prev = ab; // The command finishing clears the capture: that change applies (its // `None` is what clears the chip at the prompt). - let d = s.feed(b"\x1b]133;D;0\x07").shell.unwrap(); + let d = s.feed(b"\x1b]133;D;0\x07").shell.pop().unwrap(); assert!(shell_mark_capture_changed(&prev, &d)); } @@ -3239,13 +3300,13 @@ mod tests { let mut s = OscSniffer::new(); let sig = s.feed(b"\x1b]133;V;1\x07"); assert!( - sig.shell.is_none(), + sig.shell.is_empty(), "edit-mode metadata must not bump prompt state or prompt sequence" ); let b = s.feed(b"\x1b]133;B\x07"); - assert!(b.shell.as_ref().unwrap().active); - assert!(b.shell.as_ref().unwrap().at_prompt); + assert!(b.shell.last().unwrap().active); + assert!(b.shell.last().unwrap().at_prompt); } /// The foreground-command predicate: only a process group *other* than the @@ -3274,18 +3335,20 @@ mod tests { // The remote fish draws its prompt: A (start) then B (input begins). let mut signals = s.feed(b"\x1b]133;A\x1b]133;B\x07"); assert!( - signals.shell.as_ref().unwrap().at_prompt, + signals.shell.last().unwrap().at_prompt, "the raw marks read as at-prompt" ); // The reader consults the foreground gate before reporting. With ssh (a // different process group) on the PTY, the prompt flag is cleared. let ssh_running = is_foreground_command(Some(2000), Some(1000)); - if signals.shell.as_ref().is_some_and(|st| st.at_prompt) && ssh_running { - signals.shell.as_mut().unwrap().at_prompt = false; + if signals.shell.iter().any(|st| st.at_prompt) && ssh_running { + for st in signals.shell.iter_mut() { + st.at_prompt = false; + } } assert!( - !signals.shell.as_ref().unwrap().at_prompt, + !signals.shell.last().unwrap().at_prompt, "a foreground program's prompt marks must not engage the local editor" ); @@ -3293,10 +3356,12 @@ mod tests { // local prompt) keep at_prompt true — the local editor still engages. let mut local = s.feed(b"\x1b]133;A\x1b]133;B\x07"); let shell_idle = is_foreground_command(Some(1000), Some(1000)); - if local.shell.as_ref().is_some_and(|st| st.at_prompt) && shell_idle { - local.shell.as_mut().unwrap().at_prompt = false; + if local.shell.iter().any(|st| st.at_prompt) && shell_idle { + for st in local.shell.iter_mut() { + st.at_prompt = false; + } } - assert!(local.shell.as_ref().unwrap().at_prompt); + assert!(local.shell.last().unwrap().at_prompt); } /// Regression: a well-formed OSC marker directly following an *unterminated* @@ -3315,7 +3380,7 @@ mod tests { let mut s = OscSniffer::new(); let sig = s.feed(b"\x1b]133;A\x1b]133;B\x07"); assert!( - sig.shell.as_ref().map(|sh| sh.at_prompt).unwrap_or(false), + sig.shell.last().is_some_and(|sh| sh.at_prompt), "OSC 133;B after an unterminated 133;A was dropped (no resync on `]`)" ); @@ -3345,13 +3410,13 @@ mod tests { let mut s = OscSniffer::new(); // A command was running… - assert!(!s.feed(b"\x1b]133;C\x07").shell.as_ref().unwrap().at_prompt); + assert!(!s.feed(b"\x1b]133;C\x07").shell.last().unwrap().at_prompt); // …then finishes: D (in its own chunk, before any prompt text) already // marks us back at the prompt. let d = s.feed(b"\x1b]133;D;0\x07"); assert!( - d.shell.as_ref().unwrap().at_prompt, + d.shell.last().unwrap().at_prompt, "D should mark us back at the prompt before the prompt text is drawn" ); @@ -3363,12 +3428,12 @@ mod tests { b"\x1b]133;A\x07\x1b]7;file://host/repo/tty7\x07\r\ntty7 git:(main) \xe2\x9e\x9c ", ); assert!( - chunk.shell.as_ref().unwrap().at_prompt, + chunk.shell.last().unwrap().at_prompt, "prompt visible but at_prompt=false — the mis-routing window is still open" ); // The trailing B finally arrives and keeps it true. - assert!(s.feed(b"\x1b]133;B\x07").shell.as_ref().unwrap().at_prompt); + assert!(s.feed(b"\x1b]133;B\x07").shell.last().unwrap().at_prompt); } /// `pty_size` never reports a zero dimension (a 0×0 window would make the @@ -3499,16 +3564,16 @@ mod tests { let mut s = OscSniffer::new(); // D with no code. let d = s.feed(b"\x1b]133;D\x07"); - assert!(d.shell.as_ref().unwrap().at_prompt); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, None); + assert!(d.shell.last().unwrap().at_prompt); + assert_eq!(d.shell.last().unwrap().last_exit_code, None); // D with a non-numeric code stays None. let d = s.feed(b"\x1b]133;D;oops\x07"); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, None); + assert_eq!(d.shell.last().unwrap().last_exit_code, None); // A negative exit code parses. let d = s.feed(b"\x1b]133;D;-1\x07"); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, Some(-1)); + assert_eq!(d.shell.last().unwrap().last_exit_code, Some(-1)); } /// A fresh `PaneState` for the PTY-less state-machine tests. @@ -4119,12 +4184,12 @@ mod tests { apply_signals( &mut st, SniffSignals { - shell: Some(ShellState { + shell: vec![ShellState { active: true, at_prompt: true, last_exit_code: Some(0), command: None, - }), + }], ..SniffSignals::default() }, ); diff --git a/src/terminal/completion.rs b/src/terminal/completion.rs index 737a97c4..1e81ecc9 100644 --- a/src/terminal/completion.rs +++ b/src/terminal/completion.rs @@ -1,10 +1,15 @@ //! A small, self-contained completion engine for the command editor — tty7's own //! engine, not the shell's `compsys`. //! -//! It offers two sources, each candidate carrying the exact char range it +//! It offers three sources, each candidate carrying the exact char range it //! replaces: //! - **command** — builtins + `$PATH` executables, in command position; -//! - **path** — files / directories, elsewhere (replace just the word). +//! - **path** — files / directories, elsewhere (replace just the word); +//! - **remote path** — the same, for a pane whose filesystem is on the far +//! end of an SSH connection. The listing itself is a network round-trip the +//! view owns, so this module only splits the word into a request +//! ([`remote_path_request`]) and turns the answer into candidates +//! ([`remote_path_candidates`]) — both pure, both unit-tested. //! //! History deliberately does *not* feed the menu: //! whole-line recall belongs to the inline ghost text (frecency-ranked, cwd @@ -280,6 +285,150 @@ fn sort_candidates_by_closeness(cands: &mut [Candidate]) { }); } +/// What a path-position Tab in a remote pane needs listed on the *far side*, +/// produced by [`remote_path_request`] and consumed by +/// [`remote_path_candidates`] once the listing comes back. +/// +/// Split in two because the listing is a network round-trip: nothing here +/// touches a filesystem, so both halves stay pure and testable while the view +/// owns the async middle. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemotePathRequest { + /// Absolute directory to list on the remote. + pub dir: String, + /// What an entry's name must start with to be offered. + pub prefix: String, + /// The typed text up to and including the last `/`, re-prepended to every + /// candidate so the path the user typed is preserved (as [`complete_path`] + /// does locally). + pub dir_part: String, + /// Char range in the line the candidates replace. + pub word_start: usize, + pub cursor: usize, + /// Drop file entries — the command only takes directories. + pub dirs_only: bool, +} + +/// One entry of a remote directory listing, reduced to what completion cares +/// about. Keeps this module free of the daemon's SFTP protocol types; the view +/// converts (and is where "a symlink to a directory counts as a directory" +/// gets decided, since only the protocol knows the link target). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemoteEntry { + pub name: String, + pub is_dir: bool, +} + +/// The remote directory a path-position Tab wants listed, or `None` when the +/// caret isn't somewhere a remote path listing could help. +/// +/// `remote_cwd` is the pane's cwd *in the remote's namespace* — the caller must +/// have established that the pane really is remote. Declines: +/// - **command position** (a bare first word): those complete from `$PATH`, +/// and this machine's `$PATH` is the wrong answer for a remote anyway — +/// that's [`complete_command`]'s call, not a filesystem question. +/// - **`~`-prefixed words**: expanding one needs the remote's `$HOME`, which +/// no OSC reports. Declining hands the Tab to the remote shell, which can +/// expand it. +/// - a **relative `remote_cwd`**: nothing to resolve against. +/// +/// Separators are `/` only — deliberately not [`std::path::is_separator`], +/// which also accepts `\` on Windows. A Windows host talking to a POSIX remote +/// must not treat a backslash in the *remote's* path as a separator. +pub fn remote_path_request( + line: &str, + cursor: usize, + remote_cwd: &str, +) -> Option { + if !remote_cwd.starts_with('/') { + return None; + } + let chars: Vec = line.chars().collect(); + let cursor = cursor.min(chars.len()); + let mut word_start = cursor; + while word_start > 0 && !chars[word_start - 1].is_whitespace() { + word_start -= 1; + } + let word: String = chars[word_start..cursor].iter().collect(); + + let is_command = chars[..word_start].iter().all(|c| c.is_whitespace()); + if is_command && !word.contains('/') { + return None; + } + if word.starts_with('~') { + return None; + } + + let (dir_part, prefix) = match word.rfind('/') { + Some(i) => (&word[..=i], &word[i + 1..]), + None => ("", word.as_str()), + }; + // An absolute `dir_part` stands alone; anything else resolves against the + // remote cwd. `.`/`..` inside the path are left for the far side to + // resolve — an SFTP server handles them, and we have no remote filesystem + // to normalize against here. + let dir = if dir_part.starts_with('/') { + dir_part.to_string() + } else if dir_part.is_empty() { + remote_cwd.to_string() + } else { + format!("{}/{dir_part}", remote_cwd.trim_end_matches('/')) + }; + + Some(RemotePathRequest { + dir, + prefix: prefix.to_string(), + dir_part: dir_part.to_string(), + word_start, + cursor, + dirs_only: current_command(&chars, word_start) + .is_some_and(|c| DIR_ONLY_COMMANDS.contains(&c.as_str())), + }) +} + +/// Turn a remote directory listing into candidates for `req`. Mirrors +/// [`complete_path`]'s rules exactly — hidden entries only when the prefix asks +/// for them, `dirs_only` filtering, closeness ordering, the same cap — so a +/// remote Tab behaves like a local one. +/// +/// `.` and `..` are dropped: a local `read_dir` never yields them, and offering +/// them here would make the two panes feel different. +pub fn remote_path_candidates(req: &RemotePathRequest, entries: &[RemoteEntry]) -> Vec { + let mut out: Vec = Vec::new(); + for entry in entries { + let name = entry.name.as_str(); + if name == "." || name == ".." { + continue; + } + if name.starts_with('.') && !req.prefix.starts_with('.') { + continue; + } + if !name.starts_with(&req.prefix) { + continue; + } + if req.dirs_only && !entry.is_dir { + continue; + } + out.push(Candidate { + text: format!("{}{name}", req.dir_part), + kind: if entry.is_dir { + CandidateKind::Dir + } else { + CandidateKind::File + }, + start: req.word_start, + end: req.cursor, + description: None, + icon: None, + }); + if out.len() >= MAX_CANDIDATES { + break; + } + } + sort_candidates_by_closeness(&mut out); + out +} + /// Filesystem path completion. Splits `word` into the directory part (kept /// verbatim in each candidate so the typed path prefix is preserved) and the /// final-segment prefix to match in that directory. Ordered by closeness. @@ -1158,6 +1307,124 @@ mod tests { } } + /// The word under the caret, split and resolved against the *remote* cwd. + /// This is the request the pane's SSH connection is asked to list. + #[test] + fn remote_path_request_splits_the_word_and_resolves_against_the_remote_cwd() { + // Bare word: list the cwd itself, nothing to re-prepend. + let r = remote_path_request("cat fi", 6, "/home/me").unwrap(); + assert_eq!( + (r.dir.as_str(), r.prefix.as_str(), r.dir_part.as_str()), + ("/home/me", "fi", "") + ); + assert_eq!((r.word_start, r.cursor), (4, 6)); + assert!(!r.dirs_only); + + // Relative subdirectory: resolved against the cwd, typed text preserved. + let r = remote_path_request("cat sub/fi", 10, "/home/me").unwrap(); + assert_eq!(r.dir, "/home/me/sub/"); + assert_eq!((r.prefix.as_str(), r.dir_part.as_str()), ("fi", "sub/")); + + // Absolute: stands alone, the cwd is irrelevant. + let r = remote_path_request("cat /etc/pa", 11, "/home/me").unwrap(); + assert_eq!(r.dir, "/etc/"); + assert_eq!(r.prefix, "pa"); + + // A trailing separator on the cwd must not double up. + let r = remote_path_request("cat sub/", 8, "/").unwrap(); + assert_eq!(r.dir, "/sub/"); + + // `cd` takes directories only — same rule as the local engine. + assert!( + remote_path_request("cd pro", 6, "/home/me") + .unwrap() + .dirs_only + ); + + // A backslash is a filename character on a POSIX remote, not a + // separator — even when tty7 itself runs on Windows. + let r = remote_path_request(r"cat a\b", 7, "/home/me").unwrap(); + assert_eq!((r.dir.as_str(), r.prefix.as_str()), ("/home/me", r"a\b")); + } + + /// Positions where a remote listing is the wrong answer: the caller falls + /// back to the shell handoff for these rather than guessing. + #[test] + fn remote_path_request_declines_where_a_listing_cannot_help() { + // Command position: `$PATH`, not a directory listing. + assert!(remote_path_request("ls", 2, "/home/me").is_none()); + assert!(remote_path_request("", 0, "/home/me").is_none()); + // ...unless the "command" is itself a path, which is a real listing. + assert!(remote_path_request("./scr", 5, "/home/me").is_some()); + + // `~` needs the remote's $HOME, which no OSC reports. The remote shell + // can expand it; we can't, so we decline and let it have the Tab. + assert!(remote_path_request("cat ~/pro", 9, "/home/me").is_none()); + + // No absolute cwd to resolve against (the remote shell hasn't reported + // one yet, or reported something unusable). + assert!(remote_path_request("cat fi", 6, "").is_none()); + assert!(remote_path_request("cat fi", 6, "relative/dir").is_none()); + } + + /// A remote listing becomes candidates under exactly the local rules — + /// hidden entries stay hidden, the typed directory prefix is preserved, and + /// the ordering is the shared closeness sort. + #[test] + fn remote_path_candidates_mirror_the_local_path_rules() { + let entries = |names: &[(&str, bool)]| -> Vec { + names + .iter() + .map(|(n, d)| RemoteEntry { + name: (*n).to_string(), + is_dir: *d, + }) + .collect() + }; + let all = entries(&[ + (".", true), + ("..", true), + (".hidden", false), + ("src", true), + ("setup.py", false), + ("s", false), + ("other", false), + ]); + let texts = + |cands: Vec| -> Vec { cands.into_iter().map(|c| c.text).collect() }; + + let req = remote_path_request("cat s", 5, "/home/me").unwrap(); + let got = texts(remote_path_candidates(&req, &all)); + assert_eq!( + got, + vec!["s", "src", "setup.py"], + "prefix-matched, shortest first; `.`/`..`/hidden/non-matching dropped" + ); + + // The directory kind survives, so the menu can mark it and the insert + // can add the trailing separator. + let cands = remote_path_candidates(&req, &all); + assert!(cands.iter().find(|c| c.text == "src").unwrap().is_dir()); + assert!(!cands.iter().find(|c| c.text == "s").unwrap().is_dir()); + + // The typed directory part is re-prepended to every candidate, and the + // replacement range covers the whole word. + let req = remote_path_request("cat sub/s", 9, "/home/me").unwrap(); + let c = &remote_path_candidates(&req, &all)[0]; + assert_eq!(c.text, "sub/s"); + assert_eq!((c.start, c.end), (4, 9)); + + // A dot prefix opts into hidden entries, as it does locally. + let req = remote_path_request("cat .h", 6, "/home/me").unwrap(); + let got = texts(remote_path_candidates(&req, &all)); + assert_eq!(got, vec![".hidden"]); + + // `cd` drops the files. + let req = remote_path_request("cd s", 4, "/home/me").unwrap(); + let got = texts(remote_path_candidates(&req, &all)); + assert_eq!(got, vec!["src"]); + } + #[test] fn no_candidates_returns_none() { let dir = temp_tree("empty", &[("zzz", false)]); diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 12bfa183..7fa9d42c 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -348,6 +348,11 @@ pub struct TerminalView { /// when it opened. Typing/Backspace re-filter it in place; it closes on /// accept, on Escape, or once the edited word no longer matches anything. completion: Option, + /// Whether a remote directory listing for completion is on the wire (see + /// [`Self::spawn_remote_path_completion`]). Holding Tab down would + /// otherwise dial the daemon once per repeat while the first answer is + /// still travelling. + remote_completion_inflight: bool, /// Monotonic tag bumped every time a completion session opens or closes. /// Dynamic generators run on background threads and land their results here /// via `cx.spawn`; each task captures the generation it was spawned under and @@ -1171,6 +1176,7 @@ impl TerminalView { completion: None, completion_generation: 0, editor_handoff: None, + remote_completion_inflight: false, reverse_search: None, integration_notice: None, integration_notice_shown: false, @@ -3794,6 +3800,12 @@ impl TerminalView { let line = self.cmd.text(); let cursor = self.cmd.cursor(); let Some(comp) = super::completion::complete(&line, cursor, cwd.as_deref()) else { + // Nothing *locally*. A native-SSH pane can still answer for the + // remote filesystem over its own connection — ask before giving up + // the line (see `spawn_remote_path_completion`). + if self.spawn_remote_path_completion(&line, cursor, forward, cx) { + return; + } // Nothing to offer. Don't swallow the keypress (#136) — hand the // line to the shell and let its completion have the Tab. self.handoff_tab_to_shell(!forward, cx); @@ -3807,15 +3819,6 @@ impl TerminalView { // classic behavior byte-for-byte. let has_pending = !comp.pending.is_empty(); - if !has_pending && comp.candidates.len() == 1 { - // Unique match: accept it outright. - let c = comp.candidates[0].clone(); - self.completion_insert(&c, c.start); - self.cursor_visible = true; - cx.notify(); - return; - } - // The word range is carried by any candidate; with none (pure-generator // slot) derive it from the caret so the session still knows what it // replaces. @@ -3823,26 +3826,18 @@ impl TerminalView { Some(c) => (c.start, c.end), None => (word_start_of(&line, cursor), cursor), }; - let word: String = line - .chars() - .skip(word_start) - .take(word_end - word_start) - .collect(); - let s = CompletionSession::new(word_start, word.clone(), comp.candidates); - if !has_pending - && let Some(lcp) = s.common_prefix() - && lcp.chars().count() > word.chars().count() - { - // Static-only: fill the longest common prefix when it extends the - // typed word. All candidates share it, so the fill never invalidates - // the set. With generators pending we skip this — the eventual set - // may share a shorter prefix, and mutating the line before results - // arrive would be jarring. - self.apply_candidate(&line, word_start, word_end, &lcp); - } - let generation = self.open_completion(s); - self.cursor_visible = true; - cx.notify(); + let Some(generation) = self.offer_candidates( + &line, + word_start, + word_end, + comp.candidates, + has_pending, + cx, + ) else { + // A unique match was accepted outright; nothing is open to merge into, + // and a static-only slot has no generators anyway. + return; + }; // Kick off each generator on the background executor and merge results // back on the main thread, tagged with this session's generation. @@ -3868,6 +3863,168 @@ impl TerminalView { } } + /// Put `cands` in front of the user: accept a unique match outright, else + /// open the menu over them (filling the longest common prefix first). + /// Returns the opened session's generation, or `None` when a unique match + /// was accepted and no menu exists. + /// + /// `has_pending` means more candidates are still inbound, which disables + /// both shortcuts: a result landing a moment later could add to or change + /// the pick, and mutating the line before then would be jarring. + fn offer_candidates( + &mut self, + line: &str, + word_start: usize, + word_end: usize, + cands: Vec, + has_pending: bool, + cx: &mut Context, + ) -> Option { + if !has_pending && cands.len() == 1 { + let c = cands[0].clone(); + self.completion_insert(&c, c.start); + self.cursor_visible = true; + cx.notify(); + return None; + } + let word: String = line + .chars() + .skip(word_start) + .take(word_end - word_start) + .collect(); + let s = CompletionSession::new(word_start, word.clone(), cands); + if !has_pending + && let Some(lcp) = s.common_prefix() + && lcp.chars().count() > word.chars().count() + { + // Fill the longest common prefix when it extends the typed word. + // All candidates share it, so the fill never invalidates the set. + self.apply_candidate(line, word_start, word_end, &lcp); + } + let generation = self.open_completion(s); + self.cursor_visible = true; + cx.notify(); + Some(generation) + } + + /// The pane's cwd as a path on the *remote* — `Some` only for a native-SSH + /// pane whose remote shell has reported an absolute cwd (OSC 7). Those are + /// the panes tty7 itself dialled, so the daemon holds an authenticated + /// connection we can ask about that filesystem; every other pane kind + /// (a foreground `ssh`, WSL, plain local) answers `None`. + fn remote_ssh_cwd(&self) -> Option { + let remote = self.terminal.remote_context()?; + if remote.kind != crate::daemon::protocol::RemoteKind::NativeSsh { + return None; + } + let cwd = self.cwd()?.to_string_lossy().into_owned(); + cwd.starts_with('/').then_some(cwd) + } + + /// Complete a path against the *remote* filesystem, over the pane's own SSH + /// connection. Returns whether a request went out — the caller then leaves + /// the Tab to us instead of handing the line to the shell. + /// + /// The listing is a daemon round-trip (`SftpList` on the pane's existing + /// authenticated connection — the same channel the SFTP panel browses + /// with), so it cannot answer this keystroke synchronously. Results land on + /// the main thread and only *then* behave as a local Tab would; see + /// [`Self::remote_path_results`] for what happens to a stale or empty one. + /// + /// Out-of-band deliberately. The other way to read a remote directory is to + /// inject a listing command into the live shell and scrape it back out of + /// the PTY — the only option for a terminal that merely *bootstrapped into* + /// a session someone else dialled. tty7 opened this connection itself, so it + /// can just ask: nothing is echoed into the scrollback, no prompt hooks need + /// suppressing, and there's no stray background process to cancel when the + /// user hits Enter. The in-band route stays the answer for the pane kinds + /// that have no tty7-owned connection (a foreground `ssh`, WSL), which is + /// why those decline in [`Self::remote_ssh_cwd`] rather than pretend. + fn spawn_remote_path_completion( + &mut self, + line: &str, + cursor: usize, + forward: bool, + cx: &mut Context, + ) -> bool { + let Some(cwd) = self.remote_ssh_cwd() else { + return false; + }; + let Some(req) = completion::remote_path_request(line, cursor, &cwd) else { + return false; + }; + // A listing for this same keystroke is already on the wire. Swallow the + // repeat rather than dialling again — the answer is about to arrive and + // will open the menu. + if self.remote_completion_inflight { + return true; + } + self.remote_completion_inflight = true; + let pane_id = self.pane_id; + let dir = req.dir.clone(); + let line = line.to_string(); + cx.spawn(async move |this, cx| { + let listed = cx + .background_spawn(async move { RemoteTerminal::sftp_list(pane_id, &dir) }) + .await; + let _ = this.update(cx, |view, cx| { + view.remote_completion_inflight = false; + view.remote_path_results( + req, + &line, + cursor, + listed.unwrap_or_default(), + forward, + cx, + ) + }); + }) + .detach(); + true + } + + /// Land a remote directory listing as completion candidates. + /// + /// Three outcomes, in the order they're checked: + /// - **the line moved on** while the network answered: drop it. The + /// answer describes a word the user is no longer typing, and the Tab + /// that asked for it is long past. + /// - **nothing matched**: fall back to the shell handoff, exactly as a + /// local no-match does (#136). A directory we couldn't read, or a + /// prefix with no entries, is then no worse off than before this + /// existed — the remote's own completion still gets its shot. + /// - **candidates**: offer them like any local Tab. + fn remote_path_results( + &mut self, + req: completion::RemotePathRequest, + line: &str, + cursor: usize, + listed: Vec, + forward: bool, + cx: &mut Context, + ) { + if self.cmd.text() != line || self.cmd.cursor() != cursor { + return; + } + let entries: Vec = listed + .into_iter() + .map(|e| completion::RemoteEntry { + // Follow symlinks when classifying, as the local path engine + // does: a link to a directory takes the trailing `/` and + // survives a dirs-only filter (`cd` into one is routine). The + // daemon resolved the target for us. + is_dir: e.kind == crate::daemon::protocol::SftpEntryKind::Dir || e.target_is_dir, + name: e.name, + }) + .collect(); + let cands = completion::remote_path_candidates(&req, &entries); + if cands.is_empty() { + self.handoff_tab_to_shell(!forward, cx); + return; + } + self.offer_candidates(line, req.word_start, req.cursor, cands, false, cx); + } + /// Open a completion menu and bump the generation tag, returning it so a /// caller spawning generators can stamp their in-flight results. Every open /// gets a fresh generation, so a slow generator from a prior session can't be diff --git a/src/ui/presets.rs b/src/ui/presets.rs index 3f951cf2..80e94963 100644 --- a/src/ui/presets.rs +++ b/src/ui/presets.rs @@ -942,7 +942,8 @@ fn default_image_opacity() -> f32 { fn load_yaml_theme(path: &std::path::Path) -> Result { let text = std::fs::read_to_string(path).map_err(|e| e.to_string())?; - let file: ThemeFile = serde_yaml::from_str(&text).map_err(|e| e.to_string())?; + let file: ThemeFile = + serde_yaml::from_str(crate::core::config::strip_bom(&text)).map_err(|e| e.to_string())?; let (id, derived_name) = id_and_name(path); let background = file.background.into_fill()?;