From f668f320b8a88c6290b7234df5189e46f9ac8e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 01:50:58 -0400 Subject: [PATCH] T18 round 2 fixes: a folder the terminal cannot enter stops the start, and links read combining clusters whole --- Cargo.lock | 1 + Cargo.toml | 3 +++ TASKS.md | 3 ++- src/terminal/frame.rs | 19 ++++++++++++------- src/terminal/input.rs | 25 ++++++++++++++++++++----- src/terminal/pty.rs | 24 ++++++++++++++++++++++++ src/terminal/tests.rs | 12 +++++++++++- 7 files changed, 73 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee98e65..8e9f0f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1600,6 +1600,7 @@ dependencies = [ "raw-window-handle", "regex", "rust-embed", + "rustix 1.1.5", "serde", "serde_json", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index da42b8f..14e36da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,9 @@ cocoa = "0.26" objc = "0.2" raw-window-handle = "0.6" +[target.'cfg(unix)'.dependencies] +rustix = { version = "1", default-features = false, features = ["std", "fs"] } + [dev-dependencies] env_logger = "0.11" diff --git a/TASKS.md b/TASKS.md index 8bac575..5677cd4 100644 --- a/TASKS.md +++ b/TASKS.md @@ -240,4 +240,5 @@ Per-component lines live in `tasks/`. Tags there: | T17d | 2 | PASS | | | T17e | 1 | PASS | | | T18 | 1 | FAIL | a missing folder ran in the app's own; combining marks dropped; Tab skipped the terminal; its wheel scrolled the page too; clear kept find matches; New bash reused the old shell; the dark script typed into the replay | -| T18 | 2 | — | pending review | +| T18 | 2 | FAIL | a folder it could not enter still started the shell elsewhere; links read cluster cells as spaces | +| T18 | 3 | — | pending review | diff --git a/src/terminal/frame.rs b/src/terminal/frame.rs index e3de176..47801c7 100644 --- a/src/terminal/frame.rs +++ b/src/terminal/frame.rs @@ -49,19 +49,24 @@ pub(crate) struct Frame { pub cursor: Option<(usize, usize, CursorShape)>, } +impl Row { + /// Each cell's text, a combining cluster whole. + pub(crate) fn cells(&self) -> Vec { + let mut cells: Vec = self.text.chars().map(String::from).collect(); + for (column, cluster, _) in &self.clusters { + cells[*column] = cluster.clone(); + } + cells + } +} + impl Frame { /// The screen's text, trailing spaces gone. pub(crate) fn text(&self) -> String { let lines: Vec = self .rows .iter() - .map(|row| { - let mut cells: Vec = row.text.chars().map(String::from).collect(); - for (column, cluster, _) in &row.clusters { - cells[*column] = cluster.clone(); - } - cells.concat().trim_end().to_string() - }) + .map(|row| row.cells().concat().trim_end().to_string()) .collect(); lines.join("\n").trim_end().to_string() } diff --git a/src/terminal/input.rs b/src/terminal/input.rs index f86f8e6..5740c23 100644 --- a/src/terminal/input.rs +++ b/src/terminal/input.rs @@ -144,11 +144,26 @@ impl Terminal { } /// The link a row shows at `column`: its columns and target. - fn link_at(&self, row: usize, column: usize) -> Option<(usize, Range, Target)> { - let text = &self.shown.rows.get(row)?.text; - links(text).into_iter().find_map(|(bytes, target)| { - let start = text[..bytes.start].chars().count(); - let end = start + text[bytes].chars().count(); + pub(crate) fn link_at( + &self, + row: usize, + column: usize, + ) -> Option<(usize, Range, Target)> { + let cells = self.shown.rows.get(row)?.cells(); + let text = cells.concat(); + let column_of = |byte: usize| { + let mut from = 0; + cells + .iter() + .position(|cell| { + let past = from >= byte; + from += cell.len(); + past + }) + .unwrap_or(cells.len()) + }; + links(&text).into_iter().find_map(|(bytes, target)| { + let (start, end) = (column_of(bytes.start), column_of(bytes.end)); (start..end) .contains(&column) .then_some((row, start..end, target)) diff --git a/src/terminal/pty.rs b/src/terminal/pty.rs index 3abf02a..2521900 100644 --- a/src/terminal/pty.rs +++ b/src/terminal/pty.rs @@ -55,6 +55,10 @@ pub(crate) fn spawn(launch: &Launch, size: TermSize, cell: (u16, u16)) -> anyhow "a terminal cannot start in {}: no such folder", cwd.display() ); + #[cfg(unix)] + rustix::fs::access(cwd, rustix::fs::Access::EXEC_OK).map_err(|error| { + anyhow::anyhow!("a terminal cannot enter {}: {error}", cwd.display()) + })?; } let (sender, events) = unbounded(); let listener = Listener(sender); @@ -110,6 +114,26 @@ mod tests { use super::*; + #[test] + fn a_folder_that_cannot_be_entered_stops_the_start() { + use std::os::unix::fs::PermissionsExt; + let folder = std::env::temp_dir().join(format!("ely-shut-{}", std::process::id())); + std::fs::create_dir_all(&folder).expect("a temp folder"); + std::fs::set_permissions(&folder, std::fs::Permissions::from_mode(0o600)) + .expect("mode 600"); + let launch = Launch { + program: Some(("/bin/sh".into(), vec!["-c".into(), "true".into()])), + cwd: Some(folder.clone()), + ..Launch::default() + }; + let started = spawn(&launch, TermSize::new(20, 4), (8, 16)); + std::fs::set_permissions(&folder, std::fs::Permissions::from_mode(0o700)) + .expect("mode 700"); + std::fs::remove_dir(&folder).expect("the temp folder goes"); + let error = started.err().expect("a folder it cannot enter is an error"); + assert!(error.to_string().contains("ely-shut")); + } + #[test] fn a_missing_folder_stops_the_start() { let launch = Launch { diff --git a/src/terminal/tests.rs b/src/terminal/tests.rs index 18e85f1..41cc45e 100644 --- a/src/terminal/tests.rs +++ b/src/terminal/tests.rs @@ -1,6 +1,6 @@ use gpui::{Entity, TestAppContext, VisualTestContext}; -use super::Terminal; +use super::{Target, Terminal}; use crate::{editor::FindOptions, theme::Theme}; fn replayed<'a>( @@ -69,3 +69,13 @@ fn clearing_drops_the_matches(cx: &mut TestAppContext) { assert_eq!(terminal.matches(), (0, None)); }); } + +#[gpui::test] +fn a_link_keeps_its_combining_marks(cx: &mut TestAppContext) { + let (terminal, cx) = replayed("a https://e.dev/cafe\u{301}/x".as_bytes(), cx); + terminal.read_with(cx, |terminal, _| { + let (_, columns, target) = terminal.link_at(0, 10).expect("a link under column 10"); + assert_eq!(columns, 2..22); + assert_eq!(target, Target::Url("https://e.dev/cafe\u{301}/x".into())); + }); +}