From e505d6bf5b4f137ed2d8386856d9c575d3239d98 Mon Sep 17 00:00:00 2001 From: Julia Ryan Date: Wed, 5 Mar 2025 15:56:51 -0800 Subject: [PATCH] Git uncommit warning (#25977) Adds a prompt when clicking the uncommit button when the current commit is already present on a remote branch: ![screenshot showing prompt](https://github.com/user-attachments/assets/d6421875-588e-4db0-aee0-a92f36bce94b) Release Notes: - N/A --------- Co-authored-by: Conrad --- crates/collab/src/rpc.rs | 1 + crates/git/src/repository.rs | 58 +++++++++++++++++++++++++++++- crates/git_ui/src/git_panel.rs | 64 ++++++++++++++++++++++++---------- crates/project/src/git.rs | 50 ++++++++++++++++++++++++++ crates/proto/proto/zed.proto | 13 +++++++ crates/proto/src/proto.rs | 4 +++ 6 files changed, 171 insertions(+), 19 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 042364ac84ae830a7ef188ec6de1135dcff7dbbe..fc6290837b8100d6b015e3541c8d760b1b25a7e8 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -407,6 +407,7 @@ impl Server { .add_request_handler(forward_mutating_project_request::) .add_request_handler(forward_mutating_project_request::) .add_request_handler(forward_mutating_project_request::) + .add_request_handler(forward_mutating_project_request::) .add_message_handler(broadcast_project_message_from_host::) .add_message_handler(update_context) .add_request_handler({ diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 15406cb6c2e740eb54c031f624f197e841968c5d..3c7af56f979fdf5d4795d278add9e99643642141 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -202,8 +202,12 @@ pub trait GitRepository: Send + Sync { options: Option, ) -> Result; fn pull(&self, branch_name: &str, upstream_name: &str) -> Result; - fn get_remotes(&self, branch_name: Option<&str>) -> Result>; fn fetch(&self) -> Result; + + fn get_remotes(&self, branch_name: Option<&str>) -> Result>; + + /// returns a list of remote branches that contain HEAD + fn check_for_pushed_commit(&self) -> Result>; } #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, JsonSchema)] @@ -781,6 +785,54 @@ impl GitRepository for RealGitRepository { )); } } + + fn check_for_pushed_commit(&self) -> Result> { + let working_directory = self.working_directory()?; + let git_cmd = |args: &[&str]| -> Result { + let output = new_std_command(&self.git_binary_path) + .current_dir(&working_directory) + .args(args) + .output()?; + if output.status.success() { + Ok(String::from_utf8(output.stdout)?) + } else { + Err(anyhow!(String::from_utf8_lossy(&output.stderr).to_string())) + } + }; + + let head = git_cmd(&["rev-parse", "HEAD"]) + .context("Failed to get HEAD")? + .trim() + .to_owned(); + + let mut remote_branches = vec![]; + let mut add_if_matching = |remote_head: &str| { + if let Ok(merge_base) = git_cmd(&["merge-base", &head, remote_head]) { + if merge_base.trim() == head { + if let Some(s) = remote_head.strip_prefix("refs/remotes/") { + remote_branches.push(s.to_owned().into()); + } + } + } + }; + + // check the main branch of each remote + let remotes = git_cmd(&["remote"]).context("Failed to get remotes")?; + for remote in remotes.lines() { + if let Ok(remote_head) = + git_cmd(&["symbolic-ref", &format!("refs/remotes/{remote}/HEAD")]) + { + add_if_matching(remote_head.trim()); + } + } + + // ... and the remote branch that the checked-out one is tracking + if let Ok(remote_head) = git_cmd(&["rev-parse", "--symbolic-full-name", "@{u}"]) { + add_if_matching(remote_head.trim()); + } + + Ok(remote_branches) + } } #[cfg(not(windows))] @@ -998,6 +1050,10 @@ impl GitRepository for FakeGitRepository { fn get_remotes(&self, _branch: Option<&str>) -> Result> { unimplemented!() } + + fn check_for_pushed_commit(&self) -> Result> { + unimplemented!() + } } fn check_path_to_repo_path_errors(relative_file_path: &Path) -> Result<()> { diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 09eec43413db837b91503dab682de350b3738faa..98b88e6125521a70ff59abb3d346bab90166ecda 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -1265,34 +1265,20 @@ impl GitPanel { return; }; - // TODO: Use git merge-base to find the upstream and main branch split - let confirmation = Task::ready(true); - // let confirmation = if self.commit_editor.read(cx).is_empty(cx) { - // Task::ready(true) - // } else { - // let prompt = window.prompt( - // PromptLevel::Warning, - // "Uncomitting will replace the current commit message with the previous commit's message", - // None, - // &["Ok", "Cancel"], - // cx, - // ); - // cx.spawn(|_, _| async move { prompt.await.is_ok_and(|i| i == 0) }) - // }; - + let confirmation = self.check_for_pushed_commits(window, cx); let prior_head = self.load_commit_details("HEAD", cx); let task = cx.spawn_in(window, |this, mut cx| async move { let result = maybe!(async { - if !confirmation.await { - Ok(None) - } else { + if let Ok(true) = confirmation.await { let prior_head = prior_head.await?; repo.update(&mut cx, |repo, _| repo.reset("HEAD^", ResetMode::Soft))? .await??; Ok(Some(prior_head)) + } else { + Ok(None) } }) .await; @@ -1315,6 +1301,48 @@ impl GitPanel { self.pending_commit = Some(task); } + fn check_for_pushed_commits( + &mut self, + window: &mut Window, + cx: &mut Context, + ) -> impl Future> { + let repo = self.active_repository.clone(); + let mut cx = window.to_async(cx); + + async move { + let Some(repo) = repo else { + return Err(anyhow::anyhow!("No active repository")); + }; + + let pushed_to: Vec = repo + .update(&mut cx, |repo, _| repo.check_for_pushed_commits())? + .await??; + + if pushed_to.is_empty() { + Ok(true) + } else { + #[derive(strum::EnumIter, strum::VariantNames)] + #[strum(serialize_all = "title_case")] + enum CancelUncommit { + Uncommit, + Cancel, + } + let detail = format!( + "This commit was already pushed to {}.", + pushed_to.into_iter().join(", ") + ); + let result = cx + .update(|window, cx| prompt("Are you sure?", Some(&detail), window, cx))? + .await?; + + match result { + CancelUncommit::Cancel => Ok(false), + CancelUncommit::Uncommit => Ok(true), + } + } + } + } + /// Suggests a commit message based on the changed files and their statuses pub fn suggest_commit_message(&self) -> Option { if self.total_staged_count() != 1 { diff --git a/crates/project/src/git.rs b/crates/project/src/git.rs index d5d266ac9355b7c9ccd9980fbafa8e5a4b5418ea..fcd3bb95625631120c8de4b89d8903d091038dbc 100644 --- a/crates/project/src/git.rs +++ b/crates/project/src/git.rs @@ -110,6 +110,7 @@ impl GitStore { client.add_entity_request_handler(Self::handle_checkout_files); client.add_entity_request_handler(Self::handle_open_commit_message_buffer); client.add_entity_request_handler(Self::handle_set_index_text); + client.add_entity_request_handler(Self::handle_check_for_pushed_commits); } pub fn active_repository(&self) -> Option> { @@ -627,6 +628,29 @@ impl GitStore { }) } + async fn handle_check_for_pushed_commits( + this: Entity, + envelope: TypedEnvelope, + mut cx: AsyncApp, + ) -> Result { + let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); + let work_directory_id = ProjectEntryId::from_proto(envelope.payload.work_directory_id); + let repository_handle = + Self::repository_for_request(&this, worktree_id, work_directory_id, &mut cx)?; + + let branches = repository_handle + .update(&mut cx, |repository_handle, _| { + repository_handle.check_for_pushed_commits() + })? + .await??; + Ok(proto::CheckForPushedCommitsResponse { + pushed_to: branches + .into_iter() + .map(|commit| commit.to_string()) + .collect(), + }) + } + fn repository_for_request( this: &Entity, worktree_id: WorktreeId, @@ -1423,4 +1447,30 @@ impl Repository { } }) } + + pub fn check_for_pushed_commits(&self) -> oneshot::Receiver>> { + self.send_job(|repo| async move { + match repo { + GitRepo::Local(git_repository) => git_repository.check_for_pushed_commit(), + GitRepo::Remote { + project_id, + client, + worktree_id, + work_directory_id, + } => { + let response = client + .request(proto::CheckForPushedCommits { + project_id: project_id.0, + worktree_id: worktree_id.to_proto(), + work_directory_id: work_directory_id.to_proto(), + }) + .await?; + + let branches = response.pushed_to.into_iter().map(Into::into).collect(); + + Ok(branches) + } + } + }) + } } diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index 75e8f871d91c6ec386e0573a812a44a66e682c94..6f0d66ff9abb5acac90bd59dc2733e181a5b3ac2 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -336,6 +336,9 @@ message Envelope { GitGetBranches git_get_branches = 312; GitCreateBranch git_create_branch = 313; GitChangeBranch git_change_branch = 314; // current max + + CheckForPushedCommits check_for_pushed_commits = 315; + CheckForPushedCommitsResponse check_for_pushed_commits_response = 316; // current max } reserved 87 to 88; @@ -2875,3 +2878,13 @@ message GitChangeBranch { uint64 work_directory_id = 3; string branch_name = 4; } + +message CheckForPushedCommits { + uint64 project_id = 1; + uint64 worktree_id = 2; + uint64 work_directory_id = 3; +} + +message CheckForPushedCommitsResponse { + repeated string pushed_to = 1; +} diff --git a/crates/proto/src/proto.rs b/crates/proto/src/proto.rs index d887c6f83be7d68b00186c5206e9f3813de24dd0..9e3b3381148b40cc14051fb2f2b5ebf8adee2ec2 100644 --- a/crates/proto/src/proto.rs +++ b/crates/proto/src/proto.rs @@ -454,6 +454,8 @@ messages!( (RemoteMessageResponse, Background), (GitCreateBranch, Background), (GitChangeBranch, Background), + (CheckForPushedCommits, Background), + (CheckForPushedCommitsResponse, Background), ); request_messages!( @@ -598,6 +600,7 @@ request_messages!( (Pull, RemoteMessageResponse), (GitCreateBranch, Ack), (GitChangeBranch, Ack), + (CheckForPushedCommits, CheckForPushedCommitsResponse), ); entity_messages!( @@ -701,6 +704,7 @@ entity_messages!( Pull, GitChangeBranch, GitCreateBranch, + CheckForPushedCommits, ); entity_messages!(