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
6 changes: 5 additions & 1 deletion scripts/install-cli-command.sh
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ if [ "\${1:-}" = "browse" ]; then
done
fi

open "\$browse_url" >/dev/null 2>&1 &
# Foreground, not backgrounded: under cmux the \`open\` shim hands the URL to a
# \`cmux browser open\` child, and if this wrapper exits first the child is orphaned,
# loses its pane/session context, and the shim falls back to the system browser.
# \`open\` returns as soon as it dispatches the URL, so foregrounding costs nothing.
open "\$browse_url" >/dev/null 2>&1
exit 0
fi

Expand Down
3 changes: 2 additions & 1 deletion scripts/validate-launch-runners.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ try {
assertIncludes(ideCommand, 'browse_url="http://localhost:17877/browse?path=$(url_encode "$browse_path")"');
assertIncludes(ideCommand, 'IDE_BROWSE_PATH="$browse_path" open "$APP_BUNDLE" --args browse "$browse_path"');
assertIncludes(ideCommand, 'until running_app_reachable; do');
assertIncludes(ideCommand, 'open "$browse_url" >/dev/null 2>&1 &');
assertIncludes(ideCommand, 'open "$browse_url" >/dev/null 2>&1');
Comment thread
GordonBeeming marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
assertNotIncludes(ideCommand, 'open "$browse_url" >/dev/null 2>&1 &');
assertOrdered(ideCommand, 'if [ "${1:-}" = "browse" ]; then', 'ARGS=()');
assertIncludes(ideCommand, 'ARGS=()');
assertIncludes(ideCommand, 'ARGS+=("$(cd "$arg" && pwd -P)")');
Expand Down
115 changes: 114 additions & 1 deletion src-tauri/src/git_attribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ pub(crate) async fn attribution_for_file(workspace_root: &Path, relative: &str)
Ok(commit) => commit,
Err(_) => return unsupported(relative, "File has no local commit history"),
};
let remote_templates = remote_templates(&repo);
let remote_templates = pick_primary_template(
remote_templates(&repo),
tracking_remote_name(&repo).as_deref(),
);

let blame = match repo.blame_file(
repo_relative_bstr.as_bstr(),
Expand Down Expand Up @@ -488,6 +491,51 @@ fn github_remote_template(remote_name: &str, remote_url: &str) -> Option<RemoteT
})
}

// A repo can have several GitHub remotes — a fork's `origin` plus an unrelated second
// remote, say. A commit only lives on the remote it was pushed to, so emitting one
// generic "Open in GitHub" per remote produces duplicate buttons, some linking a repo
// that doesn't contain the commit. Resolve the branch's upstream remote so the panel can
// surface a single, correct action.
fn tracking_remote_name(repo: &gix::Repository) -> Option<String> {
let head = repo.head().ok()?;
let ref_name = head.referent_name()?;
// Read `branch.<name>.remote` directly rather than parsing the tracking ref path —
// a remote name containing a slash (e.g. `team/fork`) makes `refs/remotes/<remote>/<branch>`
// ambiguous to split. gix itself treats any slashed name as a `Name::Url` (it can't tell a
// slashed remote name from a URL either), so read the raw string rather than branching on
// the Symbol/Url distinction — the caller matches it against known remote names either way.
repo.branch_remote_name(ref_name.shorten(), gix::remote::Direction::Fetch)?
.as_bstr()
.to_str()
.ok()
.map(str::to_string)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Reduce the GitHub remotes to the single one whose commit link is meaningful: the
// branch's upstream, then `origin`, then the first as a last resort. A single remote is
// returned untouched.
fn pick_primary_template(
mut templates: Vec<RemoteTemplate>,
tracking_remote: Option<&str>,
) -> Vec<RemoteTemplate> {
if templates.len() <= 1 {
return templates;
}
let index = tracking_remote
.and_then(|name| {
templates
.iter()
.position(|template| template.remote_name == name)
})
.or_else(|| {
templates
.iter()
.position(|template| template.remote_name == "origin")
})
.unwrap_or(0);
vec![templates.swap_remove(index)]
}

fn lossless_string(value: &BStr) -> String {
value.to_str_lossy().trim().to_string()
}
Expand Down Expand Up @@ -590,6 +638,71 @@ mod tests {
assert!(github_remote_template("origin", "git@gitlab.com:org/repo.git").is_none());
}

#[test]
fn pick_primary_template_prefers_tracking_then_origin() {
let template = |name: &str| RemoteTemplate {
provider: "GitHub".to_string(),
remote_name: name.to_string(),
base_url: format!("https://github.com/acme/{name}"),
};

// A single remote is returned untouched.
assert_eq!(
pick_primary_template(vec![template("origin")], None),
vec![template("origin")]
);

// The branch's upstream wins when present.
assert_eq!(
pick_primary_template(
vec![template("origin"), template("chat-bot")],
Some("chat-bot")
),
vec![template("chat-bot")]
);

// No/unknown upstream falls back to origin.
assert_eq!(
pick_primary_template(vec![template("chat-bot"), template("origin")], None),
vec![template("origin")]
);

// No upstream and no origin falls back to the first remote.
assert_eq!(
pick_primary_template(
vec![template("chat-bot"), template("upstream")],
Some("nope")
),
vec![template("chat-bot")]
);
}

#[test]
fn tracking_remote_name_handles_a_slash_in_the_remote_name() {
let dir = tempdir().unwrap();
init_repo(dir.path());
fs::write(dir.path().join("README.md"), "first\n").unwrap();
commit_all(dir.path(), "Add readme", "1700000000 +0000");
run_git(
dir.path(),
["remote", "add", "team/fork", "git@github.com:acme/fork.git"],
);
run_git(dir.path(), ["config", "branch.main.remote", "team/fork"]);
run_git(
dir.path(),
["config", "branch.main.merge", "refs/heads/main"],
);

let repo = gix::discover(dir.path()).unwrap();

assert_eq!(
tracking_remote_name(&repo).as_deref(),
Some("team/fork"),
"a remote name containing a slash must resolve via branch.<name>.remote, \
not by splitting the refs/remotes/<remote>/<branch> tracking path"
);
}

#[tokio::test]
async fn returns_local_file_and_line_attribution() {
let dir = tempdir().unwrap();
Expand Down
16 changes: 16 additions & 0 deletions src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2100,6 +2100,22 @@ pub fn run() {
.build(tauri::generate_context!())
.expect("error while building application")
.run(|app, event| {
// A browse launch hides the `main` window and opens no visible window, so a
// dock-icon click reactivates an app with nothing to focus. Re-show + focus
// `main` (only hidden, never destroyed) so the dock icon can always summon it.
#[cfg(target_os = "macos")]
if let tauri::RunEvent::Reopen {
has_visible_windows,
..
} = &event
{
if !*has_visible_windows {
if let Some(main_window) = app.get_webview_window("main") {
focus_window(&main_window);
}
Comment thread
Copilot marked this conversation as resolved.
Comment thread
GordonBeeming marked this conversation as resolved.
}
}

#[cfg(any(target_os = "macos", target_os = "ios", target_os = "android"))]
if let tauri::RunEvent::Opened { urls } = event {
let requests = open_launch_requests_for_urls(urls);
Expand Down
Loading