Use different commit author for collab project clients (#24058)

Kirill Bulatov created

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

Change summary

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(-)

Detailed changes

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<Arc<User>> {
+        self.user_store.read(cx).current_user()
+    }
+
     pub fn remote_participants(&self) -> &BTreeMap<u64, RemoteParticipant> {
         &self.remote_participants
     }

crates/call/src/macos/room.rs 🔗

@@ -588,6 +588,10 @@ impl Room {
         &self.local_participant
     }
 
+    pub fn local_participant_user(&self, cx: &App) -> Option<Arc<User>> {
+        self.user_store.read(cx).current_user()
+    }
+
     pub fn remote_participants(&self) -> &BTreeMap<u64, RemoteParticipant> {
         &self.remote_participants
     }

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!()
     }
 }

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<Self>,
     ) {
@@ -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<Self>,
     ) {
@@ -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<Self>) {
@@ -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<Self>,
     ) -> 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))
     }
 }
 

crates/project/src/git.rs 🔗

@@ -67,8 +67,17 @@ impl PartialEq<RepositoryEntry> for RepositoryHandle {
 }
 
 enum Message {
-    StageAndCommit(GitRepo, Rope, Vec<RepoPath>),
-    Commit(GitRepo, Rope),
+    StageAndCommit {
+        git_repo: GitRepo,
+        paths: Vec<RepoPath>,
+        message: Rope,
+        name_and_email: Option<(SharedString, SharedString)>,
+    },
+    Commit {
+        git_repo: GitRepo,
+        message: Rope,
+        name_and_email: Option<(SharedString, SharedString)>,
+    },
     Stage(GitRepo, Vec<RepoPath>),
     Unstage(GitRepo, Vec<RepoPath>),
 }
@@ -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<anyhow::Error>, cx: &mut App) {
+    pub fn commit(
+        &self,
+        name_and_email: Option<(SharedString, SharedString)>,
+        mut err_sender: mpsc::Sender<anyhow::Error>,
+        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::Error>,
     ) -> 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<anyhow::Error>, cx: &mut App) {
+    pub fn commit_all(
+        &self,
+        name_and_email: Option<(SharedString, SharedString)>,
+        mut err_sender: mpsc::Sender<anyhow::Error>,
+        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::<Vec<_>>();
+            .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() {

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"))

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;
 }

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"))