From 3655961642d2c69f48c84b349e19a88aa06eda65 Mon Sep 17 00:00:00 2001 From: arookieofc <2128194521@qq.com> Date: Fri, 30 Oct 2026 15:17:11 +0800 Subject: [PATCH] feat(vcs): git UX fixes for history, remotes, changelists, branch diff and merge view (#369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Git › Show History is offered for folders, multi-selection and the project root, not just single files. - The Push dialog lets you pick any configured remote; pushing to another remote keeps the upstream's branch name and shows outgoing commits against that remote's branch. - The Changes toolbar gains New Changelist (changelists already supported new/edit/delete/set active/move). - Compare with Current gains Show Diff, opening the whole-project diff between the branch and HEAD. - Merge view: F7 / Shift+F7 navigate conflicts, the three panes scroll together, and each side pane can be annotated with Git Blame of its own content. - Windows-only test fixes in ide-lsp and ide-sdk. Fixes #343 Fixes #310 Fixes #358 Fixes #348 Fixes #320 Refs #317 Co-authored-by: a-rookie-of-C-language <186315048+a-rookie-of-C-language@users.noreply.github.com> --- crates/ide-host/src/ops/vcs/commands.rs | 29 +++++--- crates/ide-host/tests/vcs_encoding.rs | 2 +- crates/ide-lsp/src/launch.rs | 4 +- crates/ide-sdk/src/vue.rs | 4 ++ crates/ide-vcs/src/lib.rs | 32 +++++++-- crates/ide-vcs/src/write.rs | 24 ++++--- crates/ide-vcs/tests/history_ops.rs | 15 +++- src-tauri/src/commands/vcs.rs | 9 ++- src/bindings.ts | 7 +- src/editor/lsp/authorLens.ts | 2 +- src/features/project-tree/contextMenu.test.ts | 16 +++++ src/features/project-tree/contextMenu.ts | 2 +- src/features/vcs/CommitView.tsx | 1 + src/features/vcs/CompareBranches.tsx | 14 +++- src/features/vcs/MergeEditor.tsx | 71 +++++++++++++++++-- src/features/vcs/blame.ts | 23 +++--- src/features/vcs/operations.tsx | 7 ++ src/features/vcs/remoteUi.tsx | 62 ++++++++++++---- 18 files changed, 256 insertions(+), 68 deletions(-) diff --git a/crates/ide-host/src/ops/vcs/commands.rs b/crates/ide-host/src/ops/vcs/commands.rs index 3255b2043..7d3861665 100644 --- a/crates/ide-host/src/ops/vcs/commands.rs +++ b/crates/ide-host/src/ops/vcs/commands.rs @@ -45,13 +45,16 @@ pub enum RemoteOp { branch: String, }, /// Pushes local `branch` (only up to `revision` when set) to its upstream, or to - /// remote branch `target` of the same remote. + /// remote branch `target`; `remote` overrides the upstream's remote (Push dialog's + /// remote picker). PushBranch { branch: String, revision: Option, force_with_lease: bool, #[serde(default)] target: Option, + #[serde(default)] + remote: Option, }, /// Fast-forwards local `branch` (not checked out) to its upstream. UpdateBranch { @@ -198,9 +201,10 @@ pub async fn vcs_file_at(host: &Host, window: &str, path: PathBuf, rev: Option IdeResult> { +pub async fn vcs_blame(host: &Host, window: &str, path: PathBuf, text: Option) -> IdeResult> { let project = host.project(window)?; - let text = project.documents.text_of(&path); + // An explicit text wins (merge view annotates its side panes, not the worktree). + let text = text.or_else(|| project.documents.text_of(&path)); blocking(move || { project.with_repo_of(&path, |r| r.blame(&path, text.as_deref(), project.vcs_encoding(&path, None)?)) }) @@ -455,12 +459,17 @@ pub async fn vcs_remote( r.push_with_tags(force_with_lease, set_upstream, tags, &cancel, &out) } RemoteOp::DeleteBranch { branch } => r.delete_remote_branch(&branch, &cancel, &out), - RemoteOp::PushBranch { branch, revision, force_with_lease, target } => match target { - Some(target) => { - r.push_branch_to(&branch, revision.as_deref(), &target, force_with_lease, &cancel, &out) - } - None => r.push_branch(&branch, revision.as_deref(), force_with_lease, &cancel, &out), - }, + RemoteOp::PushBranch { branch, revision, force_with_lease, target, remote } => r.push_branch_to( + &branch, + PushBranchOptions { + revision: revision.as_deref(), + target: target.as_deref(), + remote: remote.as_deref(), + force_with_lease, + }, + &cancel, + &out, + ), RemoteOp::UpdateBranch { branch } => r.update_branch(&branch, &cancel, &out), RemoteOp::PullFrom { remote, branches, rebase, options } => { r.pull_from(&remote, &branches, rebase, &options, &cancel, &out) @@ -828,7 +837,7 @@ pub(crate) fn register(d: &mut Dispatcher) { d.add("vcs_branch_info", |c| async move { vcs_branch_info(&c.host, &c.window, c.arg("repo")?).await }); d.add("vcs_line_hunks", |c| async move { vcs_line_hunks(&c.host, &c.window, c.arg("path")?) }); d.add("vcs_file_at", |c| async move { vcs_file_at(&c.host, &c.window, c.arg("path")?, c.arg("rev")?).await }); - d.add("vcs_blame", |c| async move { vcs_blame(&c.host, &c.window, c.arg("path")?).await }); + d.add("vcs_blame", |c| async move { vcs_blame(&c.host, &c.window, c.arg("path")?, c.arg("text")?).await }); d.add("vcs_log", |c| async move { vcs_log(&c.host, &c.window, c.arg("repo")?, c.arg("query")?).await }); d.add("vcs_commit_files", |c| async move { vcs_commit_files(&c.host, &c.window, c.arg("repo")?, c.arg("commit")?).await diff --git a/crates/ide-host/tests/vcs_encoding.rs b/crates/ide-host/tests/vcs_encoding.rs index 0b34d18b5..32749e12e 100644 --- a/crates/ide-host/tests/vcs_encoding.rs +++ b/crates/ide-host/tests/vcs_encoding.rs @@ -96,7 +96,7 @@ async fn gbk_document_gutter_diff_stage_and_rollback_share_encoding() { project.documents.save(doc.id).unwrap(); assert_eq!(std::fs::read(&path).unwrap(), original); assert!(vcs::vcs_line_hunks(&host, "main", path.clone()).unwrap().is_empty()); - assert!(vcs::vcs_blame(&host, "main", path.clone()).await.unwrap().iter().all(|l| l.author == "Test")); + assert!(vcs::vcs_blame(&host, "main", path.clone(), None).await.unwrap().iter().all(|l| l.author == "Test")); // Missing working files still use configured encoding for historical content. project.documents.close(doc.id); let mut defaults = project.documents.file_defaults(); diff --git a/crates/ide-lsp/src/launch.rs b/crates/ide-lsp/src/launch.rs index 3343763d2..36ae556cb 100644 --- a/crates/ide-lsp/src/launch.rs +++ b/crates/ide-lsp/src/launch.rs @@ -204,7 +204,9 @@ mod tests { fn search_path_contains_env_path() { let p = search_path(); let dirs: Vec = std::env::split_paths(&p).collect(); - assert!(dirs.contains(&PathBuf::from("/usr/bin"))); + // The process's own `PATH` starts the search, then per-user and system dirs. + let first_env = std::env::split_paths(&std::env::var_os("PATH").unwrap()).find(|d| d.is_absolute()).unwrap(); + assert_eq!(dirs.first(), Some(&first_env), "{dirs:?}"); } } diff --git a/crates/ide-sdk/src/vue.rs b/crates/ide-sdk/src/vue.rs index 55f8fb00e..4c32cf997 100644 --- a/crates/ide-sdk/src/vue.rs +++ b/crates/ide-sdk/src/vue.rs @@ -72,6 +72,10 @@ mod tests { assert_eq!(found, Some(t.join(pkg).parent().unwrap().to_path_buf()), "{bin}"); } let path = std::env::join_paths([t.join("tools/node_modules/.bin")]).unwrap(); + // npm's `.bin` shims on Windows get a `.cmd` (or `.exe`) name; the bare + // name is not executable there. + let shim = if cfg!(windows) { "vue-language-server.cmd" } else { SERVER }; + touch(&t.join("tools/node_modules/.bin").join(shim)); assert_eq!(server_package(SERVER, &path), Some(t.join("tools/node_modules/@vue/language-server"))); assert_eq!(server_package("not-installed", &path), None); } diff --git a/crates/ide-vcs/src/lib.rs b/crates/ide-vcs/src/lib.rs index d5a7ebc29..30b5b414d 100644 --- a/crates/ide-vcs/src/lib.rs +++ b/crates/ide-vcs/src/lib.rs @@ -237,6 +237,19 @@ pub enum PushTags { CurrentBranch, } +/// Options of [`Repo::push_branch_to`]: which commits to push and where to. +#[derive(Clone, Copy, Debug, Default)] +pub struct PushBranchOptions<'a> { + /// Push only up to this commit of the branch (`None`: the whole branch). + pub revision: Option<&'a str>, + /// Remote branch name (`None` keeps the upstream's / the local name). + pub target: Option<&'a str>, + /// Remote to push to (`None` keeps the upstream's / the default remote). + pub remote: Option<&'a str>, + /// `git push --force-with-lease`. + pub force_with_lease: bool, +} + /// What an interactive rebase does with one commit. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Type)] #[serde(rename_all = "camelCase")] @@ -781,20 +794,25 @@ impl Repo { cancel: &CancellationToken, on_output: &dyn Fn(String), ) -> IdeResult<()> { - write::push_branch(self, branch, revision, None, force_with_lease, cancel, on_output) + write::push_branch( + self, + branch, + PushBranchOptions { revision, force_with_lease, ..Default::default() }, + cancel, + on_output, + ) } - /// [`Repo::push_branch`] to remote branch `target` instead of the upstream's name (the - /// Push dialog's editable target). + /// [`Repo::push_branch`] to remote branch `target` (None keeps the upstream's name) + /// and/or remote `remote` (None keeps the upstream's remote). The Push dialog's + /// editable target and remote picker. pub fn push_branch_to( &self, branch: &str, - revision: Option<&str>, - target: &str, - force_with_lease: bool, + options: PushBranchOptions<'_>, cancel: &CancellationToken, on_output: &dyn Fn(String), ) -> IdeResult<()> { - write::push_branch(self, branch, revision, Some(target), force_with_lease, cancel, on_output) + write::push_branch(self, branch, options, cancel, on_output) } /// Fast-forwards a local branch that is not checked out to its upstream (Branches › /// Update). Fails when the branch has diverged. diff --git a/crates/ide-vcs/src/write.rs b/crates/ide-vcs/src/write.rs index b636b1b3b..7b2747db2 100644 --- a/crates/ide-vcs/src/write.rs +++ b/crates/ide-vcs/src/write.rs @@ -4,7 +4,8 @@ use crate::cli::{self, run}; use crate::repo::{gx, resolve}; use crate::text_encoding::{decode, encode, has_bom, is_binary}; use crate::{ - BlameLine, CheckoutMode, CheckoutOptions, CheckoutOutcome, CommitOptions, DeletedBranch, DivergedLocal, Repo, + BlameLine, CheckoutMode, CheckoutOptions, CheckoutOutcome, CommitOptions, DeletedBranch, DivergedLocal, + PushBranchOptions, Repo, }; use encoding_rs::Encoding; use gix::bstr::{BStr, ByteSlice}; @@ -750,22 +751,25 @@ fn check_local_branch(r: &Repo, branch: &str) -> IdeResult<()> { .map_err(|_| IdeError::invalid(format!("'{branch}' is not a local branch"))) } -/// Pushes local `branch` (or only up to `revision`) to its upstream; a branch without -/// one goes to the default remote under the same name and, when pushed whole, gets it -/// as upstream. +/// Pushes local `branch` (or only up to `opts.revision`) to its upstream; a branch +/// without one goes to the default remote under the same name and, when pushed whole, +/// gets it as upstream. pub(crate) fn push_branch( r: &Repo, branch: &str, - revision: Option<&str>, - target: Option<&str>, - force_with_lease: bool, + opts: PushBranchOptions<'_>, cancel: &CancellationToken, on_output: &dyn Fn(String), ) -> IdeResult<()> { + let PushBranchOptions { revision, target, remote: remote_override, force_with_lease } = opts; check_local_branch(r, branch)?; - let (remote, mut name, set_upstream) = match branch_upstream(r, branch) { - Some((remote, name)) => (remote, name, false), - None => (push_remote(r)?, branch.to_string(), revision.is_none()), + let remote_override = remote_override.map(str::trim).filter(|s| !s.is_empty()); + let (remote, mut name, set_upstream) = match (remote_override, branch_upstream(r, branch)) { + // An explicit remote keeps the upstream's branch name but pushes elsewhere. + (Some(remote), Some((_, name))) => (check_rev(remote)?.to_string(), name, false), + (Some(remote), None) => (check_rev(remote)?.to_string(), branch.to_string(), revision.is_none()), + (None, Some((remote, name))) => (remote, name, false), + (None, None) => (push_remote(r)?, branch.to_string(), revision.is_none()), }; if let Some(t) = target.map(str::trim).filter(|t| !t.is_empty()) { let valid = diff --git a/crates/ide-vcs/tests/history_ops.rs b/crates/ide-vcs/tests/history_ops.rs index 9e2b756bf..16cb66d22 100644 --- a/crates/ide-vcs/tests/history_ops.rs +++ b/crates/ide-vcs/tests/history_ops.rs @@ -498,10 +498,21 @@ fn push_branch_and_up_to_revision() { assert!(repo.push_branch("nope", None, false, &cancel, &out).is_err()); // An edited target: same remote, another branch name; the upstream stays as it was. - repo.push_branch_to("main", None, "review/main", false, &cancel, &out).unwrap(); + fn opts<'a>(target: Option<&'a str>, remote: Option<&'a str>) -> PushBranchOptions<'a> { + PushBranchOptions { target, remote, ..Default::default() } + } + repo.push_branch_to("main", opts(Some("review/main"), None), &cancel, &out).unwrap(); assert_eq!(git_in(&bare, &["rev-parse", "review/main"]), c3); assert_eq!(fx.git(&["rev-parse", "--abbrev-ref", "main@{upstream}"]), "origin/main"); - assert!(repo.push_branch_to("main", None, "bad..name", false, &cancel, &out).is_err()); + assert!(repo.push_branch_to("main", opts(Some("bad..name"), None), &cancel, &out).is_err()); + + // A chosen remote: same branch name, pushed to the other remote. + let bare2 = fx._tmp.path().join("backup.git"); + git_in(fx._tmp.path(), &["init", "-q", "--bare", bare2.to_str().unwrap()]); + fx.git(&["remote", "add", "backup", bare2.to_str().unwrap()]); + repo.push_branch_to("main", opts(None, Some("backup")), &cancel, &out).unwrap(); + assert_eq!(git_in(&bare2, &["rev-parse", "main"]), c3); + assert_eq!(fx.git(&["rev-parse", "--abbrev-ref", "main@{upstream}"]), "origin/main"); } #[test] diff --git a/src-tauri/src/commands/vcs.rs b/src-tauri/src/commands/vcs.rs index 091fa5fd8..ea3488efd 100644 --- a/src-tauri/src/commands/vcs.rs +++ b/src-tauri/src/commands/vcs.rs @@ -52,8 +52,13 @@ pub async fn vcs_file_at( #[tauri::command] #[specta::specta] -pub async fn vcs_blame(window: Window, state: State<'_, AppState>, path: PathBuf) -> IdeResult> { - ide_host::ops::vcs::vcs_blame(&state.host, window.label(), path).await +pub async fn vcs_blame( + window: Window, + state: State<'_, AppState>, + path: PathBuf, + text: Option, +) -> IdeResult> { + ide_host::ops::vcs::vcs_blame(&state.host, window.label(), path, text).await } #[tauri::command] diff --git a/src/bindings.ts b/src/bindings.ts index dd16653d7..c00b882fa 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -413,7 +413,7 @@ export const commands = { /** Gutter change markers for a file, computed against its current (unsaved) text. */ vcsLineHunks: (path: string) => typedError(__TAURI_INVOKE("vcs_line_hunks", { path })), vcsFileAt: (path: string, rev: string | null) => typedError(__TAURI_INVOKE("vcs_file_at", { path, rev })), - vcsBlame: (path: string) => typedError(__TAURI_INVOKE("vcs_blame", { path })), + vcsBlame: (path: string, text: string | null) => typedError(__TAURI_INVOKE("vcs_blame", { path, text })), vcsLog: (repo: string | null, query: LogQuery) => typedError(__TAURI_INVOKE("vcs_log", { repo, query })), vcsCommitFiles: (repo: string | null, commit: string) => typedError(__TAURI_INVOKE("vcs_commit_files", { repo, commit })), /** Combined changes of several commits, `commits` oldest first. */ @@ -4865,9 +4865,10 @@ tags?: PushTags | null } | { op: "deleteBranch"; branch: string } | /** * Pushes local `branch` (only up to `revision` when set) to its upstream, or to - * remote branch `target` of the same remote. + * remote branch `target`; `remote` overrides the upstream's remote (Push dialog's + * remote picker). */ -{ op: "pushBranch"; branch: string; revision: string | null; force_with_lease: boolean; target?: string | null } | +{ op: "pushBranch"; branch: string; revision: string | null; force_with_lease: boolean; target?: string | null; remote?: string | null } | /** Fast-forwards local `branch` (not checked out) to its upstream. */ { op: "updateBranch"; branch: string } | /** Pull dialog: `branches` of `remote` (none = the configured merge branch). */ diff --git a/src/editor/lsp/authorLens.ts b/src/editor/lsp/authorLens.ts index be4392ec2..5b8c1bbb3 100644 --- a/src/editor/lsp/authorLens.ts +++ b/src/editor/lsp/authorLens.ts @@ -76,7 +76,7 @@ export function blameFor(path: string, text: Text): Promise const hit = cache.get(path); if (hit && (hit.text === text || hit.text === null)) return hit.lines; const entry: Entry = { text, lines: Promise.resolve(null) }; - entry.lines = call(commands.vcsBlame(path)).then( + entry.lines = call(commands.vcsBlame(path, null)).then( (lines) => lines, () => { entry.text = null; diff --git a/src/features/project-tree/contextMenu.test.ts b/src/features/project-tree/contextMenu.test.ts index 6fe42131a..daa6e3ba5 100644 --- a/src/features/project-tree/contextMenu.test.ts +++ b/src/features/project-tree/contextMenu.test.ts @@ -129,6 +129,22 @@ describe("project view context menu", () => { } }); + it("offers Show History for files, directories, multi-selection and the project root", () => { + const dispose = registerActions([{ id: "vcs.showHistory", title: "Show History", run: () => undefined }]); + try { + for (const nodes of [ + [node("/p/a.py")], + [node("/p/src", true)], + [node("/p/a.py"), node("/p/src", true)], + [node("/p", true, true)], + ]) { + expect(sub(contextMenuItems(nodes), "Git")).toContain("Show History"); + } + } finally { + dispose(); + } + }); + it("shows Mark Directory as for folders, without file-only items", () => { const l = labels(contextMenuItems([node(`${ROOT}/src`, true)])); expect(l).toContain("Mark Directory as"); diff --git a/src/features/project-tree/contextMenu.ts b/src/features/project-tree/contextMenu.ts index 11d660a18..7a9ddbfe1 100644 --- a/src/features/project-tree/contextMenu.ts +++ b/src/features/project-tree/contextMenu.ts @@ -332,7 +332,7 @@ export function contextMenuItems(nodes: ProjectNode[]): MenuItem[] { singleFile && act("vcs.annotate", singleFile.path, "Annotate with Git Blame"), act("vcs.showDiff", arg, "Show Diff"), singleFile && act("vcs.compareWithBranch", { path: singleFile.path }, "Compare with Branch…"), - singleFile && act("vcs.showHistory", [singleFile.path], "Show History"), + nodes.length > 0 && nodes.every((n) => !n.placeholder) && act("vcs.showHistory", arg, "Show History"), act("vcs.rollback", arg, "Rollback…"), { kind: "separator" }, act("vcs.push", undefined, "Push…"), diff --git a/src/features/vcs/CommitView.tsx b/src/features/vcs/CommitView.tsx index c650b44db..4419261c0 100644 --- a/src/features/vcs/CommitView.tsx +++ b/src/features/vcs/CommitView.tsx @@ -405,6 +405,7 @@ export function ChangesTree({ mode }: { mode: "commit" | "local" | "dialog" }) { void refreshNow()} /> {mode === "local" && } + {mode === "local" && } {mode !== "dialog" && } { + onClose(); + // Whole-project diff: every file that differs between the branches. + showCompareRevisions(branch, "HEAD", branch, current); + }, + }, + { label: "Close", role: "cancel" }, + ]} > (null); const theirsRef = useRef(null); const resultView = useRef(null); + const oursView = useRef(null); + const theirsView = useRef(null); const txt = useTxTemplate(); const titles = mergeTitles(state?.sides); @@ -188,6 +191,7 @@ export function MergeEditor({ input }: { input: EditorInput }) { const side = () => [ viewerBase({ readOnly: true }), lang, + blameSlot(), state.base !== null ? unifiedMergeView({ original: state.base, mergeControls: false, gutter: true }) : [], ]; const ours = new EditorView({ doc: state.ours, parent: oursRef.current, extensions: side() }); @@ -214,11 +218,36 @@ export function MergeEditor({ input }: { input: EditorInput }) { }); resultView.current = result; setLeft(result.state.field(conflictField).conflicts.length); + oursView.current = ours; + theirsView.current = theirs; + // Scroll the three panes together (by scroll ratio), like IDEA's merge view. + const views = [ours, result, theirs]; + let syncing = false; + const onScroll = (from: EditorView) => () => { + if (syncing) return; + syncing = true; + const s = from.scrollDOM; + const ratio = s.scrollTop / Math.max(1, s.scrollHeight - s.clientHeight); + for (const other of views) { + if (other === from) continue; + const o = other.scrollDOM; + o.scrollTop = ratio * Math.max(0, o.scrollHeight - o.clientHeight); + } + syncing = false; + }; + const offs = views.map((v) => { + const h = onScroll(v); + v.scrollDOM.addEventListener("scroll", h, { passive: true }); + return () => v.scrollDOM.removeEventListener("scroll", h); + }); return () => { + offs.forEach((off) => off()); ours.destroy(); theirs.destroy(); result.destroy(); resultView.current = null; + oursView.current = null; + theirsView.current = null; }; }, [state, lang, path]); @@ -284,6 +313,28 @@ export function MergeEditor({ input }: { input: EditorInput }) { } }; + // IDEA's merge view: F7 / Shift+F7 jump between conflicts; each side pane can + // annotate its version with Git Blame from the context menu. + const onKeyDown = (e: React.KeyboardEvent) => { + if (e.key === "F7") { + e.preventDefault(); + goto(e.shiftKey ? -1 : 1); + } + }; + + const sideMenu = (e: React.MouseEvent, viewRef: { current: EditorView | null }, text: string) => { + const view = viewRef.current; + if (!view) return; + e.preventDefault(); + showContextMenu(e.nativeEvent, [ + { + label: "Annotate with Git Blame", + icon: "changelist", + run: () => void toggleAnnotations(view, path, text), + }, + ]); + }; + if (error) return (
@@ -304,7 +355,7 @@ export function MergeEditor({ input }: { input: EditorInput }) { ); return ( -
+
goto(-1)} /> goto(1)} /> @@ -336,9 +387,21 @@ export function MergeEditor({ input }: { input: EditorInput }) {
) : null}