diff --git a/README.md b/README.md index c9aa297..3bde562 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Press `o` or Enter to open the discussion and its article. Press `c` to read the In a discussion, `j/k` moves between comments. Enter expands or collapses replies, `[/]` skips between sibling threads, and `u` returns to the parent. Page Up/Down scrolls through long comments without changing the selection. Original post text appears above the replies, with paragraphs and indented code preserved. Press `L` to choose a link from the post or selected comment. -Return to the feed with Esc; if a companion article pane is open, hn closes it (only the tab hn created — your own tabs in that pane stay open). Reopening the discussion during the same run restores loaded replies, selection, and scroll position. Press `?` for help. +Return to the feed with Esc; if a companion article pane is open, hn closes it (only the tab hn created — your own tabs in that pane stay open; if it was the last tab, the whole pane closes). Reopening the discussion during the same run restores loaded replies, selection, and scroll position. Press `?` for help. ### Save for later diff --git a/src/reader/browser.rs b/src/reader/browser.rs index 58fe085..cb72e32 100644 --- a/src/reader/browser.rs +++ b/src/reader/browser.rs @@ -143,10 +143,28 @@ fn external(url: &str) -> Result<(Option, String)> { Ok((None, "Opened in your browser".into())) } -/// Close only the article tab this CLI created. Closing the last tab shuts the -/// companion down, which also closes its Ghostty pane; tabs the user opened -/// themselves survive. A tab that is already gone (Cmd+W) counts as closed. +/// Close only the article tab this CLI created. Tabs the user opened themselves +/// survive. When the article tab is the last one, `close-tab` alone would leave +/// an empty "New tab" page in the pane, so hn instead drives the browser's own +/// close path — the same Ctrl+W byte the Ghostty keybind sends — which shuts the +/// companion down and closes its pane cleanly. A tab that is already gone +/// (Cmd+W) counts as closed. pub async fn close(target: &Target) -> Result<()> { + let tabs = control(&target.socket, json!({"cmd":"targets"})).await?; + let tabs = tabs["tabs"].as_array().context("Missing browser tabs")?; + if !tabs.iter().any(|tab| tab["id"].as_i64() == Some(target.tab)) { + return Ok(()); + } + if tabs.len() > 1 { + return close_tab(target).await; + } + if close_pane(target).await.is_ok() { + return Ok(()); + } + close_tab(target).await +} + +async fn close_tab(target: &Target) -> Result<()> { match control(&target.socket, json!({"cmd":"close-tab", "tab":target.tab})).await { Ok(_) => Ok(()), Err(e) if e.to_string().contains("no tab") => Ok(()), @@ -154,6 +172,38 @@ pub async fn close(target: &Target) -> Result<()> { } } +/// Send the pane the Ctrl+W byte (as the hn-article Cmd+W keybind does) so the +/// browser runs its own tab-close → shutdown path and the Ghostty pane exits +/// without a "Process exited" screen. +async fn close_pane(target: &Target) -> Result<()> { + let where_ = control(&target.socket, json!({"cmd":"where"})).await?; + let pane = where_["pane"].as_str().context("Browser pane unknown")?; + let output = Command::new("osascript") + .args([ + "-e", + "on run argv", + "-e", + "tell application \"Ghostty\"", + "-e", + "set browserTerminal to first terminal whose id is item 1 of argv", + "-e", + "return perform action \"text:\\x11\" on browserTerminal", + "-e", + "end tell", + "-e", + "end run", + pane, + ]) + .output() + .await + .context("osascript failed")?; + if output.status.success() { + Ok(()) + } else { + bail!("Ghostty refused the close action") + } +} + async fn focus_browser(target: &Target) { let Ok(where_) = control(&target.socket, json!({"cmd":"where"})).await else { return; diff --git a/src/reader/browser_tests.rs b/src/reader/browser_tests.rs index 0f9ab74..85c8d40 100644 --- a/src/reader/browser_tests.rs +++ b/src/reader/browser_tests.rs @@ -119,33 +119,68 @@ async fn same_story_focus_retains_tab_and_changed_story_replaces_only_owned_tab( std::fs::remove_file(path).unwrap(); } -#[tokio::test] -async fn close_sends_close_tab_for_the_owned_tab_and_treats_a_gone_tab_as_closed() { - let path = std::env::temp_dir().join(format!("hn-browser-close-{}.sock", std::process::id())); +fn spawn_close_mock( + name: &str, + tabs: Value, +) -> ( + std::path::PathBuf, + Arc>>, + tokio::task::JoinHandle<()>, +) { + let path = std::env::temp_dir().join(format!("hn-browser-{name}-{}.sock", std::process::id())); let listener = UnixListener::bind(&path).unwrap(); let requests = Arc::new(Mutex::new(Vec::::new())); let log = requests.clone(); let task = tokio::spawn(async move { loop { let Ok((stream, _)) = listener.accept().await else { break }; - let mut reader = BufReader::new(stream); - let mut line = String::new(); - reader.read_line(&mut line).await.unwrap(); - let request: Value = serde_json::from_str(&line).unwrap(); - log.lock().unwrap().push(request.clone()); - let response = if request["cmd"] == "close-tab" && request["tab"].as_i64() == Some(10) - { - json!({"id":request["id"],"ok":true,"data":{}}) - } else { - json!({"id":request["id"],"ok":false,"error":format!("no tab {}", request["tab"])}) - }; - reader - .get_mut() - .write_all(format!("{response}\n").as_bytes()) - .await - .unwrap(); + let tabs = tabs.clone(); + let log = log.clone(); + tokio::spawn(async move { + let mut reader = BufReader::new(stream); + let mut line = String::new(); + reader.read_line(&mut line).await.unwrap(); + let request: Value = serde_json::from_str(&line).unwrap(); + log.lock().unwrap().push(request.clone()); + let response = match request["cmd"].as_str().unwrap() { + "targets" => json!({"id":request["id"],"ok":true,"data":{"tabs":tabs}}), + "where" => json!({"id":request["id"],"ok":true,"data":{"pane":"99"}}), + "close-tab" if request["tab"].as_i64() == Some(10) => { + json!({"id":request["id"],"ok":true,"data":{}}) + } + "close-tab" => { + json!({"id":request["id"],"ok":false,"error":format!("no tab {}", request["tab"])}) + } + other => json!({"id":request["id"],"ok":false,"error":format!("unknown command {other}")}), + }; + reader + .get_mut() + .write_all(format!("{response}\n").as_bytes()) + .await + .unwrap(); + }); } }); + (path, requests, task) +} + +fn logged(requests: &Arc>>) -> Vec<(String, Option)> { + requests + .lock() + .unwrap() + .iter() + .map(|r| { + ( + r["cmd"].as_str().unwrap().to_owned(), + r["tab"].as_i64(), + ) + }) + .collect() +} + +#[tokio::test] +async fn close_closes_only_the_owned_tab_when_other_tabs_remain() { + let (path, requests, task) = spawn_close_mock("multi", json!([{"id":10},{"id":11}])); let target = Target { socket: path.to_str().unwrap().into(), tab: 10, @@ -153,17 +188,50 @@ async fn close_sends_close_tab_for_the_owned_tab_and_treats_a_gone_tab_as_closed close_shortcut: true, }; close(&target).await.unwrap(); - // The user may already have closed the tab with Cmd+W; that is the goal state. - let gone = Target { tab: 99, ..target }; - close(&gone).await.unwrap(); + // With other tabs present the pane must stay, so only the owned tab closes + // and the pane is never driven towards shutdown. assert_eq!( - requests - .lock() - .unwrap() - .iter() - .map(|r| (r["cmd"].as_str().unwrap().to_owned(), r["tab"].as_i64().unwrap())) - .collect::>(), - vec![("close-tab".to_owned(), 10), ("close-tab".to_owned(), 99)] + logged(&requests), + vec![("targets".to_owned(), None), ("close-tab".to_owned(), Some(10))] + ); + task.abort(); + std::fs::remove_file(path).unwrap(); +} + +#[tokio::test] +async fn close_treats_an_already_gone_tab_as_closed() { + let (path, requests, task) = spawn_close_mock("gone", json!([{"id":11}])); + let target = Target { + socket: path.to_str().unwrap().into(), + tab: 10, + story: 1, + close_shortcut: true, + }; + // The user may already have closed the tab with Cmd+W; that is the goal state. + close(&target).await.unwrap(); + assert_eq!(logged(&requests), vec![("targets".to_owned(), None)]); + task.abort(); + std::fs::remove_file(path).unwrap(); +} + +#[tokio::test] +async fn close_on_the_last_tab_still_closes_it_when_the_pane_cannot_be_driven() { + let (path, requests, task) = spawn_close_mock("last", json!([{"id":10}])); + let target = Target { + socket: path.to_str().unwrap().into(), + tab: 10, + story: 1, + close_shortcut: true, + }; + // On the last tab hn first tries to drive the pane's own close path + // (targets + where + osascript Ctrl+W); without a real Ghostty pane that + // fails and hn falls back to closing the tab itself. + close(&target).await.unwrap(); + let cmds = logged(&requests); + assert_eq!(cmds.first().unwrap().0, "targets"); + assert!( + cmds.contains(&("close-tab".to_owned(), Some(10))), + "the owned tab must still be closed, got {cmds:?}" ); task.abort(); std::fs::remove_file(path).unwrap();