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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
56 changes: 53 additions & 3 deletions src/reader/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,67 @@ fn external(url: &str) -> Result<(Option<Target>, 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(()),
Err(e) => Err(e),
}
}

/// 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;
Expand Down
126 changes: 97 additions & 29 deletions src/reader/browser_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,51 +119,119 @@ 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<Mutex<Vec<Value>>>,
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::<Value>::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<Mutex<Vec<Value>>>) -> Vec<(String, Option<i64>)> {
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,
story: 1,
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<_>>(),
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();
Expand Down
Loading