From 9a6b9e312404fc40d984e7f0acb9ba9bf0ede0d2 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 1 Feb 2025 01:25:58 +0200 Subject: [PATCH] Use different commit author for collab project clients (#24058) Follow-up of https://github.com/zed-industries/zed/pull/23869 * Retrieves user + email for collab project clients and use these when such users commit Same as in https://github.com/zed-industries/zed/pull/23329, "is it the right user name and e-mail" and "how to override these" questions apply. * If this data is unavailable, forbid committing to the remote client * Forbid running related actions in git panel, if committing/writing is not permitted Release Notes: - N/A --- crates/call/src/cross_platform/room.rs | 4 + crates/call/src/macos/room.rs | 4 + crates/git/src/repository.rs | 14 ++- crates/git_ui/src/git_panel.rs | 102 ++++++++++++++----- crates/project/src/git.rs | 96 +++++++++++++---- crates/project/src/project.rs | 4 +- crates/proto/proto/zed.proto | 2 + crates/remote_server/src/headless_project.rs | 6 +- 8 files changed, 182 insertions(+), 50 deletions(-) diff --git a/crates/call/src/cross_platform/room.rs b/crates/call/src/cross_platform/room.rs index de7852a5a0..c8e8e8cbf8 100644 --- a/crates/call/src/cross_platform/room.rs +++ b/crates/call/src/cross_platform/room.rs @@ -532,6 +532,10 @@ impl Room { &self.local_participant } + pub fn local_participant_user(&self, cx: &App) -> Option> { + self.user_store.read(cx).current_user() + } + pub fn remote_participants(&self) -> &BTreeMap { &self.remote_participants } diff --git a/crates/call/src/macos/room.rs b/crates/call/src/macos/room.rs index b0c8ad9678..d7f95dd238 100644 --- a/crates/call/src/macos/room.rs +++ b/crates/call/src/macos/room.rs @@ -588,6 +588,10 @@ impl Room { &self.local_participant } + pub fn local_participant_user(&self, cx: &App) -> Option> { + self.user_store.read(cx).current_user() + } + pub fn remote_participants(&self) -> &BTreeMap { &self.remote_participants } diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index a4eb865ffd..7e8ecac9f0 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -62,7 +62,7 @@ pub trait GitRepository: Send + Sync { /// If any of the paths were previously staged but do not exist in HEAD, they will be removed from the index. fn unstage_paths(&self, paths: &[RepoPath]) -> Result<()>; - fn commit(&self, message: &str) -> Result<()>; + fn commit(&self, message: &str, name_and_email: Option<(&str, &str)>) -> Result<()>; } impl std::fmt::Debug for dyn GitRepository { @@ -283,17 +283,23 @@ impl GitRepository for RealGitRepository { Ok(()) } - fn commit(&self, message: &str) -> Result<()> { + fn commit(&self, message: &str, name_and_email: Option<(&str, &str)>) -> Result<()> { let working_directory = self .repository .lock() .workdir() .context("failed to read git work directory")? .to_path_buf(); + let mut args = vec!["commit", "--quiet", "-m", message]; + let author = name_and_email.map(|(name, email)| format!("{name} <{email}>")); + if let Some(author) = author.as_deref() { + args.push("--author"); + args.push(author); + } let cmd = new_std_command(&self.git_binary_path) .current_dir(&working_directory) - .args(["commit", "--quiet", "-m", message]) + .args(args) .status()?; if !cmd.success() { return Err(anyhow!("Failed to commit: {cmd}")); @@ -444,7 +450,7 @@ impl GitRepository for FakeGitRepository { unimplemented!() } - fn commit(&self, _message: &str) -> Result<()> { + fn commit(&self, _message: &str, _name_and_email: Option<(&str, &str)>) -> Result<()> { unimplemented!() } } diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 8b1fe69a6f..e928a4b9c4 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -576,6 +576,7 @@ impl GitPanel { fn commit_changes( &mut self, _: &git::CommitChanges, + name_and_email: Option<(SharedString, SharedString)>, _window: &mut Window, cx: &mut Context, ) { @@ -585,13 +586,14 @@ impl GitPanel { if !active_repository.can_commit(false, cx) { return; } - active_repository.commit(self.err_sender.clone(), cx); + active_repository.commit(name_and_email, self.err_sender.clone(), cx); } /// Commit all changes, regardless of whether they are staged or not fn commit_all_changes( &mut self, _: &git::CommitAllChanges, + name_and_email: Option<(SharedString, SharedString)>, _window: &mut Window, cx: &mut Context, ) { @@ -601,7 +603,7 @@ impl GitPanel { if !active_repository.can_commit(true, cx) { return; } - active_repository.commit_all(self.err_sender.clone(), cx); + active_repository.commit_all(name_and_email, self.err_sender.clone(), cx); } fn fill_co_authors(&mut self, _: &FillCoAuthors, window: &mut Window, cx: &mut Context) { @@ -638,7 +640,7 @@ impl GitPanel { .remote_participants() .values() .filter(|participant| participant.can_write()) - .map(|participant| participant.user.clone()) + .map(|participant| participant.user.as_ref()) .filter_map(|user| { let email = user.email.as_deref()?; let name = user.name.as_deref().unwrap_or(&user.github_login); @@ -963,7 +965,8 @@ impl GitPanel { pub fn render_commit_editor( &self, - has_write_access: bool, + name_and_email: Option<(SharedString, SharedString)>, + can_commit: bool, cx: &Context, ) -> impl IntoElement { let editor = self.commit_editor.clone(); @@ -973,8 +976,8 @@ impl GitPanel { .as_ref() .map_or((false, false), |active_repository| { ( - has_write_access && active_repository.can_commit(false, cx), - has_write_access && active_repository.can_commit(true, cx), + can_commit && active_repository.can_commit(false, cx), + can_commit && active_repository.can_commit(true, cx), ) }); @@ -994,9 +997,12 @@ impl GitPanel { ) }) .disabled(!can_commit) - .on_click(cx.listener(|this, _: &ClickEvent, window, cx| { - this.commit_changes(&CommitChanges, window, cx) - })); + .on_click({ + let name_and_email = name_and_email.clone(); + cx.listener(move |this, _: &ClickEvent, window, cx| { + this.commit_changes(&CommitChanges, name_and_email.clone(), window, cx) + }) + }); let commit_all_button = self .panel_button("commit-all-changes", "Commit All") @@ -1011,9 +1017,12 @@ impl GitPanel { ) }) .disabled(!can_commit_all) - .on_click(cx.listener(|this, _: &ClickEvent, window, cx| { - this.commit_all_changes(&CommitAllChanges, window, cx) - })); + .on_click({ + let name_and_email = name_and_email.clone(); + cx.listener(move |this, _: &ClickEvent, window, cx| { + this.commit_all_changes(&CommitAllChanges, name_and_email.clone(), window, cx) + }) + }); div().w_full().h(px(140.)).px_2().pt_1().pb_2().child( v_flex() @@ -1311,22 +1320,46 @@ impl Render for GitPanel { let has_write_access = room .as_ref() .map_or(true, |room| room.read(cx).local_participant().can_write()); + let (can_commit, name_and_email) = match &room { + Some(room) => { + if project.is_via_collab() { + if has_write_access { + let name_and_email = + room.read(cx).local_participant_user(cx).and_then(|user| { + let email = SharedString::from(user.email.clone()?); + let name = user + .name + .clone() + .map(SharedString::from) + .unwrap_or(SharedString::from(user.github_login.clone())); + Some((name, email)) + }); + (name_and_email.is_some(), name_and_email) + } else { + (false, None) + } + } else { + (has_write_access, None) + } + } + None => (has_write_access, None), + }; - let has_co_authors = room.map_or(false, |room| { - has_write_access - && room - .read(cx) + let has_co_authors = can_commit + && has_write_access + && room.map_or(false, |room| { + room.read(cx) .remote_participants() .values() .any(|remote_participant| remote_participant.can_write()) - }); + }); v_flex() .id("git_panel") .key_context(self.dispatch_context(window, cx)) .track_focus(&self.focus_handle) .on_modifiers_changed(cx.listener(Self::handle_modifiers_changed)) - .when(!project.is_read_only(cx), |this| { + .when(has_write_access && !project.is_read_only(cx), |this| { this.on_action(cx.listener(|this, &ToggleStaged, window, cx| { this.toggle_staged_for_selected(&ToggleStaged, window, cx) })) @@ -1341,12 +1374,31 @@ impl Render for GitPanel { .on_action(cx.listener(|this, &RevertAll, window, cx| { this.discard_all(&RevertAll, window, cx) })) - .on_action(cx.listener(|this, &CommitChanges, window, cx| { - this.commit_changes(&CommitChanges, window, cx) - })) - .on_action(cx.listener(|this, &CommitAllChanges, window, cx| { - this.commit_all_changes(&CommitAllChanges, window, cx) - })) + .when(can_commit, |git_panel| { + git_panel + .on_action({ + let name_and_email = name_and_email.clone(); + cx.listener(move |git_panel, &CommitChanges, window, cx| { + git_panel.commit_changes( + &CommitChanges, + name_and_email.clone(), + window, + cx, + ) + }) + }) + .on_action({ + let name_and_email = name_and_email.clone(); + cx.listener(move |git_panel, &CommitAllChanges, window, cx| { + git_panel.commit_all_changes( + &CommitAllChanges, + name_and_email.clone(), + window, + cx, + ) + }) + }) + }) }) .when(self.is_focused(window, cx), |this| { this.on_action(cx.listener(Self::select_first)) @@ -1385,7 +1437,7 @@ impl Render for GitPanel { self.render_empty_state(cx).into_any_element() }) .child(self.render_divider(cx)) - .child(self.render_commit_editor(has_write_access, cx)) + .child(self.render_commit_editor(name_and_email, can_commit, cx)) } } diff --git a/crates/project/src/git.rs b/crates/project/src/git.rs index 4bd8806778..bc2bda97e0 100644 --- a/crates/project/src/git.rs +++ b/crates/project/src/git.rs @@ -67,8 +67,17 @@ impl PartialEq for RepositoryHandle { } enum Message { - StageAndCommit(GitRepo, Rope, Vec), - Commit(GitRepo, Rope), + StageAndCommit { + git_repo: GitRepo, + paths: Vec, + message: Rope, + name_and_email: Option<(SharedString, SharedString)>, + }, + Commit { + git_repo: GitRepo, + message: Rope, + name_and_email: Option<(SharedString, SharedString)>, + }, Stage(GitRepo, Vec), Unstage(GitRepo, Vec), } @@ -95,11 +104,21 @@ impl GitState { .background_executor() .spawn(async move { match msg { - Message::StageAndCommit(repo, message, paths) => { - match repo { + Message::StageAndCommit { + git_repo, + message, + name_and_email, + paths, + } => { + match git_repo { GitRepo::Local(repo) => { repo.stage_paths(&paths)?; - repo.commit(&message.to_string())?; + repo.commit( + &message.to_string(), + name_and_email.as_ref().map(|(name, email)| { + (name.as_ref(), email.as_ref()) + }), + )?; } GitRepo::Remote { project_id, @@ -119,12 +138,15 @@ impl GitState { }) .await .context("sending stage request")?; + let (name, email) = name_and_email.unzip(); client .request(proto::Commit { project_id: project_id.0, worktree_id: worktree_id.to_proto(), work_directory_id: work_directory_id.to_proto(), message: message.to_string(), + name: name.map(String::from), + email: email.map(String::from), }) .await .context("sending commit request")?; @@ -183,15 +205,25 @@ impl GitState { } Ok(()) } - Message::Commit(repo, message) => { - match repo { - GitRepo::Local(repo) => repo.commit(&message.to_string())?, + Message::Commit { + git_repo, + message, + name_and_email, + } => { + match git_repo { + GitRepo::Local(repo) => repo.commit( + &message.to_string(), + name_and_email + .as_ref() + .map(|(name, email)| (name.as_ref(), email.as_ref())), + )?, GitRepo::Remote { project_id, client, worktree_id, work_directory_id, } => { + let (name, email) = name_and_email.unzip(); client .request(proto::Commit { project_id: project_id.0, @@ -200,6 +232,8 @@ impl GitState { // TODO implement collaborative commit message buffer instead and use it // If it works, remove `commit_with_message` method. message: message.to_string(), + name: name.map(String::from), + email: email.map(String::from), }) .await .context("sending commit request")?; @@ -453,14 +487,24 @@ impl RepositoryHandle { && (commit_all || self.have_staged_changes()); } - pub fn commit(&self, mut err_sender: mpsc::Sender, cx: &mut App) { + pub fn commit( + &self, + name_and_email: Option<(SharedString, SharedString)>, + mut err_sender: mpsc::Sender, + cx: &mut App, + ) { let Some(git_repo) = self.git_repo.clone() else { return; }; let message = self.commit_message.read(cx).as_rope().clone(); - let result = self - .update_sender - .unbounded_send((Message::Commit(git_repo, message), err_sender.clone())); + let result = self.update_sender.unbounded_send(( + Message::Commit { + git_repo, + message, + name_and_email, + }, + err_sender.clone(), + )); if result.is_err() { cx.spawn(|_| async move { err_sender @@ -479,19 +523,30 @@ impl RepositoryHandle { pub fn commit_with_message( &self, message: String, + name_and_email: Option<(SharedString, SharedString)>, err_sender: mpsc::Sender, ) -> anyhow::Result<()> { let Some(git_repo) = self.git_repo.clone() else { return Ok(()); }; - let result = self - .update_sender - .unbounded_send((Message::Commit(git_repo, message.into()), err_sender)); + let result = self.update_sender.unbounded_send(( + Message::Commit { + git_repo, + message: message.into(), + name_and_email, + }, + err_sender, + )); anyhow::ensure!(result.is_ok(), "Failed to submit commit operation"); Ok(()) } - pub fn commit_all(&self, mut err_sender: mpsc::Sender, cx: &mut App) { + pub fn commit_all( + &self, + name_and_email: Option<(SharedString, SharedString)>, + mut err_sender: mpsc::Sender, + cx: &mut App, + ) { let Some(git_repo) = self.git_repo.clone() else { return; }; @@ -500,10 +555,15 @@ impl RepositoryHandle { .status() .filter(|entry| !entry.status.is_staged().unwrap_or(false)) .map(|entry| entry.repo_path.clone()) - .collect::>(); + .collect(); let message = self.commit_message.read(cx).as_rope().clone(); let result = self.update_sender.unbounded_send(( - Message::StageAndCommit(git_repo, message, to_stage), + Message::StageAndCommit { + git_repo, + paths: to_stage, + message, + name_and_email, + }, err_sender.clone(), )); if result.is_err() { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 08db152c09..2b4fa95bd2 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4073,9 +4073,11 @@ impl Project { })??; let commit_message = envelope.payload.message; + let name = envelope.payload.name.map(SharedString::from); + let email = envelope.payload.email.map(SharedString::from); let (err_sender, mut err_receiver) = mpsc::channel(1); repository_handle - .commit_with_message(commit_message, err_sender) + .commit_with_message(commit_message, name.zip(email), err_sender) .context("unstaging entries")?; if let Some(error) = err_receiver.next().await { Err(error.context("error during unstaging")) diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index abe2be81f3..f4b36d5768 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -2656,4 +2656,6 @@ message Commit { uint64 worktree_id = 2; uint64 work_directory_id = 3; string message = 4; + optional string name = 5; + optional string email = 6; } diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index dc1f89b8e2..ba70e832c8 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -4,7 +4,7 @@ use extension_host::headless_host::HeadlessExtensionStore; use fs::Fs; use futures::channel::mpsc; use git::repository::RepoPath; -use gpui::{App, AppContext as _, AsyncApp, Context, Entity, PromptLevel}; +use gpui::{App, AppContext as _, AsyncApp, Context, Entity, PromptLevel, SharedString}; use http_client::HttpClient; use language::{proto::serialize_operation, Buffer, BufferEvent, LanguageRegistry}; use node_runtime::NodeRuntime; @@ -721,9 +721,11 @@ impl HeadlessProject { })??; let commit_message = envelope.payload.message; + let name = envelope.payload.name.map(SharedString::from); + let email = envelope.payload.email.map(SharedString::from); let (err_sender, mut err_receiver) = mpsc::channel(1); repository_handle - .commit_with_message(commit_message, err_sender) + .commit_with_message(commit_message, name.zip(email), err_sender) .context("unstaging entries")?; if let Some(error) = err_receiver.next().await { Err(error.context("error during unstaging"))