Refactor to remove toggle_follow

Conrad Irwin created

Change summary

crates/collab/src/tests/channel_buffer_tests.rs |  4 -
crates/collab/src/tests/following_tests.rs      | 30 +++++++++---------
crates/workspace/src/workspace.rs               | 22 ++++++------
3 files changed, 27 insertions(+), 29 deletions(-)

Detailed changes

crates/collab/src/tests/channel_buffer_tests.rs 🔗

@@ -702,9 +702,7 @@ async fn test_following_to_channel_notes_without_a_shared_project(
     // Client B follows client A.
     workspace_b
         .update(cx_b, |workspace, cx| {
-            workspace
-                .toggle_follow(client_a.peer_id().unwrap(), cx)
-                .unwrap()
+            workspace.follow(client_a.peer_id().unwrap(), cx).unwrap()
         })
         .await
         .unwrap();

crates/collab/src/tests/following_tests.rs 🔗

@@ -126,7 +126,7 @@ async fn test_basic_following(
     // When client B starts following client A, all visible view states are replicated to client B.
     workspace_b
         .update(cx_b, |workspace, cx| {
-            workspace.toggle_follow(peer_id_a, cx).unwrap()
+            workspace.follow(peer_id_a, cx).unwrap()
         })
         .await
         .unwrap();
@@ -166,7 +166,7 @@ async fn test_basic_following(
     // Client C also follows client A.
     workspace_c
         .update(cx_c, |workspace, cx| {
-            workspace.toggle_follow(peer_id_a, cx).unwrap()
+            workspace.follow(peer_id_a, cx).unwrap()
         })
         .await
         .unwrap();
@@ -201,7 +201,7 @@ async fn test_basic_following(
 
     // Client C unfollows client A.
     workspace_c.update(cx_c, |workspace, cx| {
-        workspace.toggle_follow(peer_id_a, cx);
+        workspace.unfollow(&workspace.active_pane().clone(), cx);
     });
 
     // All clients see that clients B is following client A.
@@ -224,7 +224,7 @@ async fn test_basic_following(
 
     // Client C re-follows client A.
     workspace_c.update(cx_c, |workspace, cx| {
-        workspace.toggle_follow(peer_id_a, cx);
+        workspace.follow(peer_id_a, cx);
     });
 
     // All clients see that clients B and C are following client A.
@@ -248,7 +248,7 @@ async fn test_basic_following(
     // Client D follows client C.
     workspace_d
         .update(cx_d, |workspace, cx| {
-            workspace.toggle_follow(peer_id_c, cx).unwrap()
+            workspace.follow(peer_id_c, cx).unwrap()
         })
         .await
         .unwrap();
@@ -439,7 +439,7 @@ async fn test_basic_following(
     // Client A starts following client B.
     workspace_a
         .update(cx_a, |workspace, cx| {
-            workspace.toggle_follow(peer_id_b, cx).unwrap()
+            workspace.follow(peer_id_b, cx).unwrap()
         })
         .await
         .unwrap();
@@ -644,7 +644,7 @@ async fn test_following_tab_order(
     //Follow client B as client A
     workspace_a
         .update(cx_a, |workspace, cx| {
-            workspace.toggle_follow(client_b_id, cx).unwrap()
+            workspace.follow(client_b_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -756,7 +756,7 @@ async fn test_peers_following_each_other(
         .update(cx_a, |workspace, cx| {
             assert_ne!(*workspace.active_pane(), pane_a1);
             let leader_id = *project_a.read(cx).collaborators().keys().next().unwrap();
-            workspace.toggle_follow(leader_id, cx).unwrap()
+            workspace.follow(leader_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -767,7 +767,7 @@ async fn test_peers_following_each_other(
         .update(cx_b, |workspace, cx| {
             assert_ne!(*workspace.active_pane(), pane_b1);
             let leader_id = *project_b.read(cx).collaborators().keys().next().unwrap();
-            workspace.toggle_follow(leader_id, cx).unwrap()
+            workspace.follow(leader_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -914,7 +914,7 @@ async fn test_auto_unfollowing(
     });
     workspace_b
         .update(cx_b, |workspace, cx| {
-            workspace.toggle_follow(leader_id, cx).unwrap()
+            workspace.follow(leader_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -939,7 +939,7 @@ async fn test_auto_unfollowing(
 
     workspace_b
         .update(cx_b, |workspace, cx| {
-            workspace.toggle_follow(leader_id, cx).unwrap()
+            workspace.follow(leader_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -957,7 +957,7 @@ async fn test_auto_unfollowing(
 
     workspace_b
         .update(cx_b, |workspace, cx| {
-            workspace.toggle_follow(leader_id, cx).unwrap()
+            workspace.follow(leader_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -977,7 +977,7 @@ async fn test_auto_unfollowing(
 
     workspace_b
         .update(cx_b, |workspace, cx| {
-            workspace.toggle_follow(leader_id, cx).unwrap()
+            workspace.follow(leader_id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -1053,10 +1053,10 @@ async fn test_peers_simultaneously_following_each_other(
     });
 
     let a_follow_b = workspace_a.update(cx_a, |workspace, cx| {
-        workspace.toggle_follow(client_b_id, cx).unwrap()
+        workspace.follow(client_b_id, cx).unwrap()
     });
     let b_follow_a = workspace_b.update(cx_b, |workspace, cx| {
-        workspace.toggle_follow(client_a_id, cx).unwrap()
+        workspace.follow(client_a_id, cx).unwrap()
     });
 
     futures::try_join!(a_follow_b, b_follow_a).unwrap();

crates/workspace/src/workspace.rs 🔗

@@ -2520,19 +2520,13 @@ impl Workspace {
         cx.notify();
     }
 
-    pub fn toggle_follow(
+    fn start_following(
         &mut self,
         leader_id: PeerId,
         cx: &mut ViewContext<Self>,
     ) -> Option<Task<Result<()>>> {
         let pane = self.active_pane().clone();
 
-        if let Some(prev_leader_id) = self.unfollow(&pane, cx) {
-            if leader_id == prev_leader_id {
-                return None;
-            }
-        }
-
         self.last_leaders_by_pane
             .insert(pane.downgrade(), leader_id);
         self.follower_states_by_leader
@@ -2603,9 +2597,15 @@ impl Workspace {
             None
         };
 
-        next_leader_id
-            .or_else(|| collaborators.keys().copied().next())
-            .and_then(|leader_id| self.toggle_follow(leader_id, cx))
+        let pane = self.active_pane.clone();
+        let Some(leader_id) = next_leader_id.or_else(|| collaborators.keys().copied().next())
+        else {
+            return None;
+        };
+        if Some(leader_id) == self.unfollow(&pane, cx) {
+            return None;
+        }
+        self.follow(leader_id, cx)
     }
 
     pub fn follow(
@@ -2654,7 +2654,7 @@ impl Workspace {
         }
 
         // Otherwise, follow.
-        self.toggle_follow(leader_id, cx)
+        self.start_following(leader_id, cx)
     }
 
     pub fn unfollow(