From 1981de4caee62954a316a825222c4fa8fa020af8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 10:48:33 -0800 Subject: [PATCH 01/88] Add REST APIs for getting and adding contributors Co-authored-by: Mikayla --- .../20221109000000_test_schema.sql | 6 ++ .../20240122174606_add_contributors.sql | 5 ++ crates/collab/src/api.rs | 24 ++++- crates/collab/src/db/queries.rs | 1 + crates/collab/src/db/queries/contributors.rs | 50 +++++++++++ crates/collab/src/db/queries/users.rs | 90 ++++++++++--------- crates/collab/src/db/tables.rs | 1 + crates/collab/src/db/tables/contributor.rs | 30 +++++++ crates/collab/src/db/tables/user.rs | 2 + crates/collab/src/db/tests.rs | 1 + .../collab/src/db/tests/contributor_tests.rs | 37 ++++++++ crates/collab/src/db/tests/db_tests.rs | 15 +--- 12 files changed, 206 insertions(+), 56 deletions(-) create mode 100644 crates/collab/migrations/20240122174606_add_contributors.sql create mode 100644 crates/collab/src/db/queries/contributors.rs create mode 100644 crates/collab/src/db/tables/contributor.rs create mode 100644 crates/collab/src/db/tests/contributor_tests.rs diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 8d8f523c94c2caa2e70212f3e08ae6f4f493599a..14657fe682eb5741a5e86b345e87987be86fe3a8 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -344,3 +344,9 @@ CREATE INDEX "index_notifications_on_recipient_id_is_read_kind_entity_id" ON "notifications" ("recipient_id", "is_read", "kind", "entity_id"); + +CREATE TABLE contributors ( + user_id INTEGER REFERENCES users(id), + signed_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (user_id) +); diff --git a/crates/collab/migrations/20240122174606_add_contributors.sql b/crates/collab/migrations/20240122174606_add_contributors.sql new file mode 100644 index 0000000000000000000000000000000000000000..16bec82d4f2bd0a1b3f4221366cd822ebcd70bb1 --- /dev/null +++ b/crates/collab/migrations/20240122174606_add_contributors.sql @@ -0,0 +1,5 @@ +CREATE TABLE contributors ( + user_id INTEGER REFERENCES users(id), + signed_at TIMESTAMP NOT NULL DEFAULT NOW(), + PRIMARY KEY (user_id) +); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 6bdbd7357fb857c4db90bfc0f5583023d3b76daf..88c813ecc33116d000641bb2db7339b57618789f 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -25,6 +25,7 @@ pub fn routes(rpc_server: Arc, state: Arc) -> Router(req: Request, next: Next) -> impl IntoR #[derive(Debug, Deserialize)] struct AuthenticatedUserParams { - github_user_id: Option, + github_user_id: i32, github_login: String, github_email: Option, } @@ -88,8 +89,7 @@ async fn get_authenticated_user( params.github_user_id, params.github_email.as_deref(), ) - .await? - .ok_or_else(|| Error::Http(StatusCode::NOT_FOUND, "user not found".into()))?; + .await?; let metrics_id = app.db.get_user_metrics_id(user.id).await?; return Ok(Json(AuthenticatedUserResponse { user, metrics_id })); } @@ -133,6 +133,24 @@ async fn get_rpc_server_snapshot( Ok(ErasedJson::pretty(rpc_server.snapshot().await)) } +async fn get_contributors(Extension(app): Extension>) -> Result>> { + Ok(Json(app.db.get_contributors().await?)) +} + +async fn add_contributor( + Json(params): Json, + Extension(app): Extension>, +) -> Result<()> { + Ok(app + .db + .add_contributor( + ¶ms.github_login, + params.github_user_id, + params.github_email.as_deref(), + ) + .await?) +} + #[derive(Deserialize)] struct CreateAccessTokenQueryParams { public_key: String, diff --git a/crates/collab/src/db/queries.rs b/crates/collab/src/db/queries.rs index 629e26f1a9e2ac1479f80984d2f9ae3efe7e9ab7..f6bba13ede5fee59b313a602fbf25d3a3d9b3ace 100644 --- a/crates/collab/src/db/queries.rs +++ b/crates/collab/src/db/queries.rs @@ -4,6 +4,7 @@ pub mod access_tokens; pub mod buffers; pub mod channels; pub mod contacts; +pub mod contributors; pub mod messages; pub mod notifications; pub mod projects; diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs new file mode 100644 index 0000000000000000000000000000000000000000..593409670b3feb6494ec6e03f566570aac01cf78 --- /dev/null +++ b/crates/collab/src/db/queries/contributors.rs @@ -0,0 +1,50 @@ +use super::*; + +impl Database { + /// Retrieves the GitHub logins of all users who have signed the CLA. + pub async fn get_contributors(&self) -> Result> { + self.transaction(|tx| async move { + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryGithubLogin { + GithubLogin, + } + + Ok(contributor::Entity::find() + .inner_join(user::Entity) + .order_by_asc(contributor::Column::SignedAt) + .select_only() + .column(user::Column::GithubLogin) + .into_values::<_, QueryGithubLogin>() + .all(&*tx) + .await?) + }) + .await + } + + /// Records that a given user has signed the CLA. + pub async fn add_contributor( + &self, + github_login: &str, + github_user_id: i32, + github_email: Option<&str>, + ) -> Result<()> { + self.transaction(|tx| async move { + let user = self + .get_or_create_user_by_github_account_tx( + github_login, + github_user_id, + github_email, + &*tx, + ) + .await?; + contributor::ActiveModel { + user_id: ActiveValue::Set(user.id), + signed_at: ActiveValue::NotSet, + } + .insert(&*tx) + .await?; + Ok(()) + }) + .await + } +} diff --git a/crates/collab/src/db/queries/users.rs b/crates/collab/src/db/queries/users.rs index d6dfe480427fc8ed6dce8f460b5b307e7735317e..4249f06617709668c54e9a4b0b870f5b3f302af0 100644 --- a/crates/collab/src/db/queries/users.rs +++ b/crates/collab/src/db/queries/users.rs @@ -72,53 +72,61 @@ impl Database { pub async fn get_or_create_user_by_github_account( &self, github_login: &str, - github_user_id: Option, + github_user_id: i32, github_email: Option<&str>, - ) -> Result> { + ) -> Result { self.transaction(|tx| async move { - let tx = &*tx; - if let Some(github_user_id) = github_user_id { - if let Some(user_by_github_user_id) = user::Entity::find() - .filter(user::Column::GithubUserId.eq(github_user_id)) - .one(tx) - .await? - { - let mut user_by_github_user_id = user_by_github_user_id.into_active_model(); - user_by_github_user_id.github_login = ActiveValue::set(github_login.into()); - Ok(Some(user_by_github_user_id.update(tx).await?)) - } else if let Some(user_by_github_login) = user::Entity::find() - .filter(user::Column::GithubLogin.eq(github_login)) - .one(tx) - .await? - { - let mut user_by_github_login = user_by_github_login.into_active_model(); - user_by_github_login.github_user_id = ActiveValue::set(Some(github_user_id)); - Ok(Some(user_by_github_login.update(tx).await?)) - } else { - let user = user::Entity::insert(user::ActiveModel { - email_address: ActiveValue::set(github_email.map(|email| email.into())), - github_login: ActiveValue::set(github_login.into()), - github_user_id: ActiveValue::set(Some(github_user_id)), - admin: ActiveValue::set(false), - invite_count: ActiveValue::set(0), - invite_code: ActiveValue::set(None), - metrics_id: ActiveValue::set(Uuid::new_v4()), - ..Default::default() - }) - .exec_with_returning(&*tx) - .await?; - Ok(Some(user)) - } - } else { - Ok(user::Entity::find() - .filter(user::Column::GithubLogin.eq(github_login)) - .one(tx) - .await?) - } + self.get_or_create_user_by_github_account_tx( + github_login, + github_user_id, + github_email, + &*tx, + ) + .await }) .await } + pub async fn get_or_create_user_by_github_account_tx( + &self, + github_login: &str, + github_user_id: i32, + github_email: Option<&str>, + tx: &DatabaseTransaction, + ) -> Result { + if let Some(user_by_github_user_id) = user::Entity::find() + .filter(user::Column::GithubUserId.eq(github_user_id)) + .one(tx) + .await? + { + let mut user_by_github_user_id = user_by_github_user_id.into_active_model(); + user_by_github_user_id.github_login = ActiveValue::set(github_login.into()); + Ok(user_by_github_user_id.update(tx).await?) + } else if let Some(user_by_github_login) = user::Entity::find() + .filter(user::Column::GithubLogin.eq(github_login)) + .one(tx) + .await? + { + let mut user_by_github_login = user_by_github_login.into_active_model(); + user_by_github_login.github_user_id = ActiveValue::set(Some(github_user_id)); + Ok(user_by_github_login.update(tx).await?) + } else { + let user = user::Entity::insert(user::ActiveModel { + email_address: ActiveValue::set(github_email.map(|email| email.into())), + github_login: ActiveValue::set(github_login.into()), + github_user_id: ActiveValue::set(Some(github_user_id)), + admin: ActiveValue::set(false), + invite_count: ActiveValue::set(0), + invite_code: ActiveValue::set(None), + metrics_id: ActiveValue::set(Uuid::new_v4()), + ..Default::default() + }) + .exec_with_returning(&*tx) + .await?; + Ok(user) + } + } + /// get_all_users returns the next page of users. To get more call again with /// the same limit and the page incremented by 1. pub async fn get_all_users(&self, page: u32, limit: u32) -> Result> { diff --git a/crates/collab/src/db/tables.rs b/crates/collab/src/db/tables.rs index 4f28ce4fbd4f6a5c3214270001efb11a6885c293..646447c91f6e3c56016786a5d39f81aa8f5e8eef 100644 --- a/crates/collab/src/db/tables.rs +++ b/crates/collab/src/db/tables.rs @@ -9,6 +9,7 @@ pub mod channel_member; pub mod channel_message; pub mod channel_message_mention; pub mod contact; +pub mod contributor; pub mod feature_flag; pub mod follower; pub mod language_server; diff --git a/crates/collab/src/db/tables/contributor.rs b/crates/collab/src/db/tables/contributor.rs new file mode 100644 index 0000000000000000000000000000000000000000..3ae96a62d910e38823828a7eb502c0d9840111c2 --- /dev/null +++ b/crates/collab/src/db/tables/contributor.rs @@ -0,0 +1,30 @@ +use crate::db::UserId; +use sea_orm::entity::prelude::*; +use serde::Serialize; + +/// A user who has signed the CLA. +#[derive(Clone, Debug, Default, PartialEq, Eq, DeriveEntityModel, Serialize)] +#[sea_orm(table_name = "contributors")] +pub struct Model { + #[sea_orm(primary_key)] + pub user_id: UserId, + pub signed_at: DateTime, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::user::Entity", + from = "Column::UserId", + to = "super::user::Column::Id" + )] + User, +} + +impl ActiveModelBehavior for ActiveModel {} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::User.def() + } +} diff --git a/crates/collab/src/db/tables/user.rs b/crates/collab/src/db/tables/user.rs index 53866b5c54f96a2e3b42c06515acba4a341bead3..5ab7f17a01ebee9073bfb66bf124936adcc0e9f5 100644 --- a/crates/collab/src/db/tables/user.rs +++ b/crates/collab/src/db/tables/user.rs @@ -31,6 +31,8 @@ pub enum Relation { ChannelMemberships, #[sea_orm(has_many = "super::user_feature::Entity")] UserFeatures, + #[sea_orm(has_one = "super::contributor::Entity")] + Contributor, } impl Related for Entity { diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index 56e37abc1d9248a885724a50dcd0a8ca25ec93dc..4a9c98f02240964fc27b462cb6764fdbedc9c567 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -1,5 +1,6 @@ mod buffer_tests; mod channel_tests; +mod contributor_tests; mod db_tests; mod feature_flag_tests; mod message_tests; diff --git a/crates/collab/src/db/tests/contributor_tests.rs b/crates/collab/src/db/tests/contributor_tests.rs new file mode 100644 index 0000000000000000000000000000000000000000..1985229f2f35a120a9db38ddf7f79bc74fad64b8 --- /dev/null +++ b/crates/collab/src/db/tests/contributor_tests.rs @@ -0,0 +1,37 @@ +use super::Database; +use crate::{db::NewUserParams, test_both_dbs}; +use std::sync::Arc; + +test_both_dbs!( + test_contributors, + test_contributors_postgres, + test_contributors_sqlite +); + +async fn test_contributors(db: &Arc) { + db.create_user( + &format!("user1@example.com"), + false, + NewUserParams { + github_login: format!("user1"), + github_user_id: 1, + }, + ) + .await + .unwrap() + .user_id; + + assert_eq!(db.get_contributors().await.unwrap(), Vec::::new()); + + db.add_contributor("user1", 1, None).await.unwrap(); + assert_eq!( + db.get_contributors().await.unwrap(), + vec!["user1".to_string()] + ); + + db.add_contributor("user2", 2, None).await.unwrap(); + assert_eq!( + db.get_contributors().await.unwrap(), + vec!["user1".to_string(), "user2".to_string()] + ); +} diff --git a/crates/collab/src/db/tests/db_tests.rs b/crates/collab/src/db/tests/db_tests.rs index 3e1bdede71e3691dc1da0e0df0fb59c6c92ac83b..a14799005bcdf8ac06c776762832baacdd7644b9 100644 --- a/crates/collab/src/db/tests/db_tests.rs +++ b/crates/collab/src/db/tests/db_tests.rs @@ -106,33 +106,24 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { .user_id; let user = db - .get_or_create_user_by_github_account("login1", None, None) + .get_or_create_user_by_github_account("login1", 1, None) .await - .unwrap() .unwrap(); assert_eq!(user.id, user_id1); assert_eq!(&user.github_login, "login1"); assert_eq!(user.github_user_id, Some(101)); - assert!(db - .get_or_create_user_by_github_account("non-existent-login", None, None) - .await - .unwrap() - .is_none()); - let user = db - .get_or_create_user_by_github_account("the-new-login2", Some(102), None) + .get_or_create_user_by_github_account("the-new-login2", 102, None) .await - .unwrap() .unwrap(); assert_eq!(user.id, user_id2); assert_eq!(&user.github_login, "the-new-login2"); assert_eq!(user.github_user_id, Some(102)); let user = db - .get_or_create_user_by_github_account("login3", Some(103), Some("user3@example.com")) + .get_or_create_user_by_github_account("login3", 103, Some("user3@example.com")) .await - .unwrap() .unwrap(); assert_eq!(&user.github_login, "login3"); assert_eq!(user.github_user_id, Some(103)); From 53b47c15ac53c4f3a44a27af6142d71e0581d098 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 22 Jan 2024 13:33:13 -0700 Subject: [PATCH 02/88] Hide editor hovers when a menu is open --- crates/editor/src/element.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index eeb8263f30fa1449350bf7872d45866cadadb54f..ee40b8e4781546e7b0eee1df7454dbe2d6749aed 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -2153,14 +2153,18 @@ impl EditorElement { .max(MIN_POPOVER_LINE_HEIGHT * line_height), // Apply minimum height of 4 lines ); - let hover = editor.hover_state.render( + let hover = if context_menu.is_some() { + None + } else { + editor.hover_state.render( &snapshot, &style, visible_rows, max_size, editor.workspace.as_ref().map(|(w, _)| w.clone()), cx, - ); + ) + }; let editor_view = cx.view().clone(); let fold_indicators = cx.with_element_context(|cx| { From 6c937c4a905ae85f8269d3e85adb3a92f91b5fdc Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 12:49:35 -0800 Subject: [PATCH 03/88] Make POST /contributors API idempotent Co-authored-by: Marshall --- crates/collab/src/db/queries/contributors.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs index 593409670b3feb6494ec6e03f566570aac01cf78..f0e352e2f2cfdc184ba7cca58c207f42d2cff402 100644 --- a/crates/collab/src/db/queries/contributors.rs +++ b/crates/collab/src/db/queries/contributors.rs @@ -37,11 +37,17 @@ impl Database { &*tx, ) .await?; - contributor::ActiveModel { + + contributor::Entity::insert(contributor::ActiveModel { user_id: ActiveValue::Set(user.id), signed_at: ActiveValue::NotSet, - } - .insert(&*tx) + }) + .on_conflict( + OnConflict::column(contributor::Column::UserId) + .do_nothing() + .to_owned(), + ) + .exec_without_returning(&*tx) .await?; Ok(()) }) From 56556d71a188861b6d7a98667a1c2867ee12cd2d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 13:11:24 -0800 Subject: [PATCH 04/88] Add API for retrieving the date that a contributor signed the CLA Co-authored-by: Marshall --- Cargo.lock | 1 + Cargo.toml | 1 + crates/assistant/Cargo.toml | 2 +- crates/collab/Cargo.toml | 1 + crates/collab/src/api.rs | 25 ++++++++++++++++++++ crates/collab/src/db/queries/contributors.rs | 22 +++++++++++++++++ 6 files changed, 51 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index df785345e5bca37a8951e5b3c234840f18869baa..516a21308f8eb18e7358eeb60ce7843331fd0f6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1463,6 +1463,7 @@ dependencies = [ "base64 0.13.1", "call", "channel", + "chrono", "clap 3.2.25", "client", "clock", diff --git a/Cargo.toml b/Cargo.toml index 79d28821d4b040f35fc1ab1dd391cddeee93bc47..eea2b4fb47711f1783a3dc4fed972846ce1b0f69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,6 +93,7 @@ resolver = "2" anyhow = { version = "1.0.57" } async-trait = { version = "0.1" } async-compression = { version = "0.4", features = ["gzip", "futures-io"] } +chrono = { version = "0.4", features = ["serde"] } ctor = "0.2.6" derive_more = { version = "0.99.17" } env_logger = { version = "0.9" } diff --git a/crates/assistant/Cargo.toml b/crates/assistant/Cargo.toml index 9588932c250edf6b7bd4194d2c4cc17ec0108bf5..5380b91a9cf7b6746c1059d948c4e334275029f7 100644 --- a/crates/assistant/Cargo.toml +++ b/crates/assistant/Cargo.toml @@ -30,7 +30,7 @@ workspace = { path = "../workspace" } uuid.workspace = true log.workspace = true anyhow.workspace = true -chrono = { version = "0.4", features = ["serde"] } +chrono.workspace = true futures.workspace = true indoc.workspace = true isahc.workspace = true diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index e30622adc3d360f40bb3c832cca063f6d4f6a84e..16ec9dbefd52d2b20353590ee58962e462fae131 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -27,6 +27,7 @@ axum = { version = "0.5", features = ["json", "headers", "ws"] } axum-extra = { version = "0.3", features = ["erased-json"] } base64 = "0.13" clap = { version = "3.1", features = ["derive"], optional = true } +chrono.workspace = true dashmap = "5.4" envy = "0.4.2" futures.workspace = true diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 88c813ecc33116d000641bb2db7339b57618789f..0eb82d29d2cf94940ac910197c483c36ad6226b7 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -14,6 +14,7 @@ use axum::{ Extension, Json, Router, }; use axum_extra::response::ErasedJson; +use chrono::SecondsFormat; use serde::{Deserialize, Serialize}; use std::sync::Arc; use tower::ServiceBuilder; @@ -26,6 +27,7 @@ pub fn routes(rpc_server: Arc, state: Arc) -> Router>) -> Result, +} + +async fn check_is_contributor( + Extension(app): Extension>, + Query(params): Query, +) -> Result> { + Ok(Json(CheckIsContributorResponse { + signed_at: app + .db + .get_contributor_sign_timestamp(params.github_user_id) + .await? + .map(|ts| ts.and_utc().to_rfc3339_opts(SecondsFormat::Millis, true)), + })) +} + async fn add_contributor( Json(params): Json, Extension(app): Extension>, diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs index f0e352e2f2cfdc184ba7cca58c207f42d2cff402..3bf0d458b154537a50d261fa9b7540be8542e2f7 100644 --- a/crates/collab/src/db/queries/contributors.rs +++ b/crates/collab/src/db/queries/contributors.rs @@ -21,6 +21,28 @@ impl Database { .await } + /// Records that a given user has signed the CLA. + pub async fn get_contributor_sign_timestamp( + &self, + github_user_id: i32, + ) -> Result> { + self.transaction(|tx| async move { + let Some(user) = user::Entity::find() + .filter(user::Column::GithubUserId.eq(github_user_id)) + .one(&*tx) + .await? + else { + return Ok(None); + }; + let Some(contributor) = contributor::Entity::find_by_id(user.id).one(&*tx).await? + else { + return Ok(None); + }; + Ok(Some(contributor.signed_at)) + }) + .await + } + /// Records that a given user has signed the CLA. pub async fn add_contributor( &self, From a9ddef8227422b576d9f4fcaa5bb030403c53bb5 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 22 Jan 2024 14:48:14 -0700 Subject: [PATCH 05/88] Update Channel membership UI * Remove ability to act on people defined in parent channels * Show promote buttons on guests --- .../src/collab_panel/channel_modal.rs | 128 +++++++++++------- 1 file changed, 79 insertions(+), 49 deletions(-) diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index e92422d76d79e7b2706aed83b892031f9b4b3a96..3d7facf2e8d917d7ea93a9f2e0e57fa306a9bf5d 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -10,10 +10,11 @@ use gpui::{ WeakView, }; use picker::{Picker, PickerDelegate}; +use rpc::proto::channel_member; use std::sync::Arc; use ui::{prelude::*, Avatar, Checkbox, ContextMenu, ListItem, ListItemSpacing}; use util::TryFutureExt; -use workspace::ModalView; +use workspace::{notifications::NotifyTaskExt, ModalView}; actions!( channel_modal, @@ -347,15 +348,13 @@ impl PickerDelegate for ChannelModalDelegate { } fn confirm(&mut self, _: bool, cx: &mut ViewContext>) { - if let Some((selected_user, role)) = self.user_at_index(self.selected_index) { + if let Some(selected_user) = self.user_at_index(self.selected_index) { if Some(selected_user.id) == self.user_store.read(cx).current_user().map(|user| user.id) { return; } match self.mode { - Mode::ManageMembers => { - self.show_context_menu(selected_user, role.unwrap_or(ChannelRole::Member), cx) - } + Mode::ManageMembers => self.show_context_menu(self.selected_index, cx), Mode::InviteMembers => match self.member_status(selected_user.id, cx) { Some(proto::channel_member::Kind::Invitee) => { self.remove_member(selected_user.id, cx); @@ -385,7 +384,8 @@ impl PickerDelegate for ChannelModalDelegate { selected: bool, cx: &mut ViewContext>, ) -> Option { - let (user, role) = self.user_at_index(ix)?; + let user = self.user_at_index(ix)?; + let membership = self.member_at_index(ix); let request_status = self.member_status(user.id, cx); let is_me = self.user_store.read(cx).current_user().map(|user| user.id) == Some(user.id); @@ -402,11 +402,15 @@ impl PickerDelegate for ChannelModalDelegate { .children( if request_status == Some(proto::channel_member::Kind::Invitee) { Some(Label::new("Invited")) + } else if membership.map(|m| m.kind) + == Some(channel_member::Kind::AncestorMember) + { + Some(Label::new("Parent")) } else { None }, ) - .children(match role { + .children(match membership.map(|m| m.role) { Some(ChannelRole::Admin) => Some(Label::new("Admin")), Some(ChannelRole::Guest) => Some(Label::new("Guest")), _ => None, @@ -419,7 +423,7 @@ impl PickerDelegate for ChannelModalDelegate { if let (Some((menu, _)), true) = (&self.context_menu, selected) { Some( overlay() - .anchor(gpui::AnchorCorner::TopLeft) + .anchor(gpui::AnchorCorner::TopRight) .child(menu.clone()), ) } else { @@ -458,16 +462,19 @@ impl ChannelModalDelegate { }) } - fn user_at_index(&self, ix: usize) -> Option<(Arc, Option)> { + fn member_at_index(&self, ix: usize) -> Option<&ChannelMembership> { + self.matching_member_indices + .get(ix) + .and_then(|ix| self.members.get(*ix)) + } + + fn user_at_index(&self, ix: usize) -> Option> { match self.mode { Mode::ManageMembers => self.matching_member_indices.get(ix).and_then(|ix| { let channel_membership = self.members.get(*ix)?; - Some(( - channel_membership.user.clone(), - Some(channel_membership.role), - )) + Some(channel_membership.user.clone()) }), - Mode::InviteMembers => Some((self.matching_users.get(ix).cloned()?, None)), + Mode::InviteMembers => self.matching_users.get(ix).cloned(), } } @@ -491,7 +498,7 @@ impl ChannelModalDelegate { cx.notify(); }) }) - .detach_and_log_err(cx); + .detach_and_notify_err(cx); Some(()) } @@ -523,7 +530,7 @@ impl ChannelModalDelegate { cx.notify(); }) }) - .detach_and_log_err(cx); + .detach_and_notify_err(cx); Some(()) } @@ -549,19 +556,66 @@ impl ChannelModalDelegate { cx.notify(); }) }) - .detach_and_log_err(cx); + .detach_and_notify_err(cx); } - fn show_context_menu( - &mut self, - user: Arc, - role: ChannelRole, - cx: &mut ViewContext>, - ) { - let user_id = user.id; + fn show_context_menu(&mut self, ix: usize, cx: &mut ViewContext>) { + let Some(membership) = self.member_at_index(ix) else { + return; + }; + if membership.kind == proto::channel_member::Kind::AncestorMember { + return; + } + let user_id = membership.user.id; let picker = cx.view().clone(); let context_menu = ContextMenu::build(cx, |mut menu, _cx| { - menu = menu.entry("Remove Member", None, { + if membership.kind == channel_member::Kind::AncestorMember { + return menu.entry("Inherited membership", None, |_| {}); + }; + + let role = membership.role; + + if role == ChannelRole::Admin || role == ChannelRole::Member { + let picker = picker.clone(); + menu = menu.entry("Demote to Guest", None, move |cx| { + picker.update(cx, |picker, cx| { + picker + .delegate + .set_user_role(user_id, ChannelRole::Guest, cx); + }) + }); + } + + if role == ChannelRole::Admin || role == ChannelRole::Guest { + let picker = picker.clone(); + let label = if role == ChannelRole::Guest { + "Promote to Member" + } else { + "Demote to Member" + }; + + menu = menu.entry(label, None, move |cx| { + picker.update(cx, |picker, cx| { + picker + .delegate + .set_user_role(user_id, ChannelRole::Member, cx); + }) + }); + } + + if role == ChannelRole::Member || role == ChannelRole::Guest { + let picker = picker.clone(); + menu = menu.entry("Promote to Admin", None, move |cx| { + picker.update(cx, |picker, cx| { + picker + .delegate + .set_user_role(user_id, ChannelRole::Admin, cx); + }) + }); + }; + + menu = menu.separator(); + menu = menu.entry("Remove from Channel", None, { let picker = picker.clone(); move |cx| { picker.update(cx, |picker, cx| { @@ -569,30 +623,6 @@ impl ChannelModalDelegate { }) } }); - - let picker = picker.clone(); - match role { - ChannelRole::Admin => { - menu = menu.entry("Revoke Admin", None, move |cx| { - picker.update(cx, |picker, cx| { - picker - .delegate - .set_user_role(user_id, ChannelRole::Member, cx); - }) - }); - } - ChannelRole::Member => { - menu = menu.entry("Make Admin", None, move |cx| { - picker.update(cx, |picker, cx| { - picker - .delegate - .set_user_role(user_id, ChannelRole::Admin, cx); - }) - }); - } - _ => {} - }; - menu }); cx.focus_view(&context_menu); From cd7f377c313ebaa058eb8263953eb8c174ed924d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 13:55:12 -0800 Subject: [PATCH 06/88] Allow checking CLA signatures by GitHub login This will be used by CLA Bot. Co-authored-by: Marshall --- crates/collab/src/api.rs | 24 +++++++++++++++++--- crates/collab/src/db.rs | 1 + crates/collab/src/db/queries/contributors.rs | 23 ++++++++++++++----- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 0eb82d29d2cf94940ac910197c483c36ad6226b7..d0cac55df1bd7aaf57c5cae0b880fdf7b877023a 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -1,6 +1,6 @@ use crate::{ auth, - db::{User, UserId}, + db::{ContributorSelector, User, UserId}, rpc, AppState, Error, Result, }; use anyhow::anyhow; @@ -141,7 +141,24 @@ async fn get_contributors(Extension(app): Extension>) -> Result, + github_login: Option, +} + +impl CheckIsContributorParams { + fn as_contributor_selector(self) -> Result { + if let Some(github_user_id) = self.github_user_id { + return Ok(ContributorSelector::GitHubUserId { github_user_id }); + } + + if let Some(github_login) = self.github_login { + return Ok(ContributorSelector::GitHubLogin { github_login }); + } + + Err(anyhow!( + "must be one of `github_user_id` or `github_login`." + ))? + } } #[derive(Debug, Serialize)] @@ -153,10 +170,11 @@ async fn check_is_contributor( Extension(app): Extension>, Query(params): Query, ) -> Result> { + let params = params.as_contributor_selector()?; Ok(Json(CheckIsContributorResponse { signed_at: app .db - .get_contributor_sign_timestamp(params.github_user_id) + .get_contributor_sign_timestamp(¶ms) .await? .map(|ts| ts.and_utc().to_rfc3339_opts(SecondsFormat::Millis, true)), })) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 480dcf6f8595143659069739104608dac4c15fd8..f3eeb68afc8db31633f549fbd40161bcf5242530 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -44,6 +44,7 @@ use tables::*; use tokio::sync::{Mutex, OwnedMutexGuard}; pub use ids::*; +pub use queries::contributors::ContributorSelector; pub use sea_orm::ConnectOptions; pub use tables::user::Model as User; diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs index 3bf0d458b154537a50d261fa9b7540be8542e2f7..30d89cab7ba4d588bc81fe461c246aa5134f3a21 100644 --- a/crates/collab/src/db/queries/contributors.rs +++ b/crates/collab/src/db/queries/contributors.rs @@ -1,5 +1,11 @@ use super::*; +#[derive(Debug)] +pub enum ContributorSelector { + GitHubUserId { github_user_id: i32 }, + GitHubLogin { github_login: String }, +} + impl Database { /// Retrieves the GitHub logins of all users who have signed the CLA. pub async fn get_contributors(&self) -> Result> { @@ -24,14 +30,19 @@ impl Database { /// Records that a given user has signed the CLA. pub async fn get_contributor_sign_timestamp( &self, - github_user_id: i32, + selector: &ContributorSelector, ) -> Result> { self.transaction(|tx| async move { - let Some(user) = user::Entity::find() - .filter(user::Column::GithubUserId.eq(github_user_id)) - .one(&*tx) - .await? - else { + let condition = match selector { + ContributorSelector::GitHubUserId { github_user_id } => { + user::Column::GithubUserId.eq(*github_user_id) + } + ContributorSelector::GitHubLogin { github_login } => { + user::Column::GithubLogin.eq(github_login) + } + }; + + let Some(user) = user::Entity::find().filter(condition).one(&*tx).await? else { return Ok(None); }; let Some(contributor) = contributor::Entity::find_by_id(user.id).one(&*tx).await? From 03e008959fea02980b53ae76c32d83bd76884d61 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 14:12:31 -0800 Subject: [PATCH 07/88] Fix get_or_create_user test --- crates/collab/src/db/tests/db_tests.rs | 31 +++++++++----------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/crates/collab/src/db/tests/db_tests.rs b/crates/collab/src/db/tests/db_tests.rs index a14799005bcdf8ac06c776762832baacdd7644b9..7ae1a8a1a45b6c0ffe2b99d8106d4b6eb66aea93 100644 --- a/crates/collab/src/db/tests/db_tests.rs +++ b/crates/collab/src/db/tests/db_tests.rs @@ -80,18 +80,17 @@ test_both_dbs!( ); async fn test_get_or_create_user_by_github_account(db: &Arc) { - let user_id1 = db - .create_user( - "user1@example.com", - false, - NewUserParams { - github_login: "login1".into(), - github_user_id: 101, - }, - ) - .await - .unwrap() - .user_id; + db.create_user( + "user1@example.com", + false, + NewUserParams { + github_login: "login1".into(), + github_user_id: 101, + }, + ) + .await + .unwrap() + .user_id; let user_id2 = db .create_user( "user2@example.com", @@ -105,14 +104,6 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { .unwrap() .user_id; - let user = db - .get_or_create_user_by_github_account("login1", 1, None) - .await - .unwrap(); - assert_eq!(user.id, user_id1); - assert_eq!(&user.github_login, "login1"); - assert_eq!(user.github_user_id, Some(101)); - let user = db .get_or_create_user_by_github_account("the-new-login2", 102, None) .await From b9458fe4ac07b6bc86b29cadeb18a3878590b8ba Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 14:15:41 -0800 Subject: [PATCH 08/88] Fix call to get_or_create_user in seed binary --- crates/collab/src/bin/seed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/collab/src/bin/seed.rs b/crates/collab/src/bin/seed.rs index ed24ccef75dce446eb54a431d1371139d67b7140..bca2a7899a6bc2f49f760ca78ff78fb58ea4994b 100644 --- a/crates/collab/src/bin/seed.rs +++ b/crates/collab/src/bin/seed.rs @@ -68,7 +68,7 @@ async fn main() { user_count += 1; db.get_or_create_user_by_github_account( &github_user.login, - Some(github_user.id), + github_user.id, github_user.email.as_deref(), ) .await From 676d2cb24ac644eb00e6a3346ea5c72a686a5f8e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 14:22:04 -0800 Subject: [PATCH 09/88] collab 0.39.0 --- Cargo.lock | 2 +- crates/collab/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 516a21308f8eb18e7358eeb60ce7843331fd0f6e..96f0e54f54c4ec234c38333fe7a3afcdd010976e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1452,7 +1452,7 @@ dependencies = [ [[package]] name = "collab" -version = "0.38.0" +version = "0.39.0" dependencies = [ "anyhow", "async-trait", diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 16ec9dbefd52d2b20353590ee58962e462fae131..3d5dfbc747b9d13deba405b19b2e648b31d7dbc8 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] default-run = "collab" edition = "2021" name = "collab" -version = "0.38.0" +version = "0.39.0" publish = false [[bin]] From 98d514f5bf2c9a53519f462e084791551dc8f69b Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 22 Jan 2024 15:21:10 -0700 Subject: [PATCH 10/88] Fix off-by-one highlighting in hover tooltip rust analyzer has a tendency to return markdown of the form: ```rust // <-- note the leading space blah blah blah ``` This is clearly defectuous, so we used to .trim() the output. Unfortunately we trim after applying syntax highlighting, so that causes the output to look goofy. Fix this by updating the highlighting when we trim. --- crates/editor/src/hover_popover.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index f311f20ae6d0faf9bc42193245fa42ae14cc5fac..a4bc1c5cdebe49f90e7f8eaa13add0de25ff498a 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -394,6 +394,27 @@ async fn parse_blocks( } } + let leading_space = text.chars().take_while(|c| c.is_whitespace()).count(); + if leading_space > 0 { + highlights = highlights + .into_iter() + .map(|(range, style)| { + ( + range.start.saturating_sub(leading_space) + ..range.end.saturating_sub(leading_space), + style, + ) + }) + .collect(); + region_ranges = region_ranges + .into_iter() + .map(|range| { + range.start.saturating_sub(leading_space) + ..range.end.saturating_sub(leading_space), + }) + .collect(); + } + ParsedMarkdown { text: text.trim().to_string(), highlights, From a75fa35a08658989fa3a66873b8a45eb458bce72 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 22 Jan 2024 20:06:21 +0200 Subject: [PATCH 11/88] Fix the fonts panic co-authored-by: Piotr --- crates/gpui/src/platform/mac/text_system.rs | 45 +++++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/crates/gpui/src/platform/mac/text_system.rs b/crates/gpui/src/platform/mac/text_system.rs index ba434026f65f9573acaf73c14c6abb21d6448b35..21fe3c70a8a8787056fd3409c837af76a70427cf 100644 --- a/crates/gpui/src/platform/mac/text_system.rs +++ b/crates/gpui/src/platform/mac/text_system.rs @@ -81,13 +81,11 @@ impl PlatformTextSystem for MacTextSystem { fn all_font_names(&self) -> Vec { let collection = core_text::font_collection::create_for_all_families(); let Some(descriptors) = collection.get_descriptors() else { - return vec![]; + return Vec::new(); }; let mut names = BTreeSet::new(); for descriptor in descriptors.into_iter() { - names.insert(descriptor.font_name()); - names.insert(descriptor.family_name()); - names.insert(descriptor.style_name()); + names.extend(lenient_font_attributes::family_name(&descriptor)); } if let Ok(fonts_in_memory) = self.0.read().memory_source.all_families() { names.extend(fonts_in_memory); @@ -612,6 +610,45 @@ impl From for FontkitStyle { } } +// Some fonts may have no attributest despite `core_text` requiring them (and panicking). +// This is the same version as `core_text` has without `expect` calls. +mod lenient_font_attributes { + use core_foundation::{ + base::{CFRetain, CFType, TCFType}, + string::{CFString, CFStringRef}, + }; + use core_text::font_descriptor::{ + kCTFontFamilyNameAttribute, CTFontDescriptor, CTFontDescriptorCopyAttribute, + }; + + pub fn family_name(descriptor: &CTFontDescriptor) -> Option { + unsafe { get_string_attribute(descriptor, kCTFontFamilyNameAttribute) } + } + + fn get_string_attribute( + descriptor: &CTFontDescriptor, + attribute: CFStringRef, + ) -> Option { + unsafe { + let value = CTFontDescriptorCopyAttribute(descriptor.as_concrete_TypeRef(), attribute); + if value.is_null() { + return None; + } + + let value = CFType::wrap_under_create_rule(value); + assert!(value.instance_of::()); + let s = wrap_under_get_rule(value.as_CFTypeRef() as CFStringRef); + Some(s.to_string()) + } + } + + unsafe fn wrap_under_get_rule(reference: CFStringRef) -> CFString { + assert!(!reference.is_null(), "Attempted to create a NULL object."); + let reference = CFRetain(reference as *const ::std::os::raw::c_void) as CFStringRef; + TCFType::wrap_under_create_rule(reference) + } +} + #[cfg(test)] mod tests { use crate::{font, px, FontRun, MacTextSystem, PlatformTextSystem}; From fd8d2d41b02168196d7369f0f4161c9358fb97c6 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 22 Jan 2024 15:53:12 -0700 Subject: [PATCH 12/88] One off --- crates/editor/src/hover_popover.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index a4bc1c5cdebe49f90e7f8eaa13add0de25ff498a..668d00f1aabcf67cd9293e1bfaf1d3abe5c57091 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -409,8 +409,7 @@ async fn parse_blocks( region_ranges = region_ranges .into_iter() .map(|range| { - range.start.saturating_sub(leading_space) - ..range.end.saturating_sub(leading_space), + range.start.saturating_sub(leading_space)..range.end.saturating_sub(leading_space) }) .collect(); } From a2aa47aba2f8d1240aa435d8e0708cff57199f0d Mon Sep 17 00:00:00 2001 From: Julia Date: Mon, 22 Jan 2024 18:12:25 -0500 Subject: [PATCH 13/88] Avoid overwriting mouse wheel scroll with selection auto-scroll --- crates/editor/src/editor.rs | 10 +++++----- crates/editor/src/element.rs | 3 +-- crates/editor/src/scroll.rs | 22 +++++++++++++++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e378425666c9550e30c18d447d67baf3b0e05c76..4be449f3cdee12fd88aece382920c1bb18f4ea1c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -281,7 +281,7 @@ pub enum SelectPhase { Update { position: DisplayPoint, goal_column: u32, - scroll_position: gpui::Point, + scroll_delta: gpui::Point, }, End, } @@ -1928,8 +1928,8 @@ impl Editor { SelectPhase::Update { position, goal_column, - scroll_position, - } => self.update_selection(position, goal_column, scroll_position, cx), + scroll_delta, + } => self.update_selection(position, goal_column, scroll_delta, cx), SelectPhase::End => self.end_selection(cx), } } @@ -2063,7 +2063,7 @@ impl Editor { &mut self, position: DisplayPoint, goal_column: u32, - scroll_position: gpui::Point, + scroll_delta: gpui::Point, cx: &mut ViewContext, ) { let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); @@ -2152,7 +2152,7 @@ impl Editor { return; } - self.set_scroll_position(scroll_position, cx); + self.apply_scroll_delta(scroll_delta, cx); cx.notify(); } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index eeb8263f30fa1449350bf7872d45866cadadb54f..519e17a5af761a546ea5941cdeb5dd6ed329cf72 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -531,8 +531,7 @@ impl EditorElement { SelectPhase::Update { position: point_for_position.previous_valid, goal_column: point_for_position.exact_unclipped.column(), - scroll_position: (position_map.snapshot.scroll_position() + scroll_delta) - .clamp(&gpui::Point::default(), &position_map.scroll_max), + scroll_delta, }, cx, ); diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index f68004109e8889600c08b91ffe1edc5cc54ddee1..3f68b7d2d991439b9548fa298ebbfd18daeeeb9d 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -327,6 +327,16 @@ impl Editor { } } + pub fn apply_scroll_delta( + &mut self, + scroll_delta: gpui::Point, + cx: &mut ViewContext, + ) { + let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); + let position = self.scroll_manager.anchor.scroll_position(&display_map) + scroll_delta; + self.set_scroll_position_taking_display_map(position, true, false, display_map, cx); + } + pub fn set_scroll_position( &mut self, scroll_position: gpui::Point, @@ -343,12 +353,22 @@ impl Editor { cx: &mut ViewContext, ) { let map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); + self.set_scroll_position_taking_display_map(scroll_position, local, autoscroll, map, cx); + } + fn set_scroll_position_taking_display_map( + &mut self, + scroll_position: gpui::Point, + local: bool, + autoscroll: bool, + display_map: DisplaySnapshot, + cx: &mut ViewContext, + ) { hide_hover(self, cx); let workspace_id = self.workspace.as_ref().map(|workspace| workspace.1); self.scroll_manager.set_scroll_position( scroll_position, - &map, + &display_map, local, autoscroll, workspace_id, From 25708088b7ef72b791f35722f370625f8da043ae Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 16:38:54 -0800 Subject: [PATCH 14/88] Add requires_zed_cla column to channels table Don't allow granting guests write access in a call where the channel or one of its ancestors requires the zed CLA, until that guest has signed the Zed CLA. Co-authored-by: Marshall --- .../20221109000000_test_schema.sql | 3 +- ...dd_requires_zed_cla_column_to_channels.sql | 1 + crates/collab/src/api.rs | 2 +- crates/collab/src/bin/seed.rs | 2 +- crates/collab/src/db/ids.rs | 8 ++ crates/collab/src/db/queries/channels.rs | 17 +++ crates/collab/src/db/queries/contributors.rs | 2 +- crates/collab/src/db/queries/rooms.rs | 44 ++++++++ crates/collab/src/db/queries/users.rs | 69 ++++++------ crates/collab/src/db/tables/channel.rs | 1 + .../collab/src/db/tests/contributor_tests.rs | 4 +- crates/collab/src/db/tests/db_tests.rs | 4 +- .../collab/src/tests/channel_guest_tests.rs | 102 +++++++++++++++++- crates/collab/src/tests/test_server.rs | 6 +- 14 files changed, 225 insertions(+), 40 deletions(-) create mode 100644 crates/collab/migrations/20240122224506_add_requires_zed_cla_column_to_channels.sql diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 14657fe682eb5741a5e86b345e87987be86fe3a8..95237b22f1de052f929e52b6bde1a96d7e5d2403 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -196,7 +196,8 @@ CREATE TABLE "channels" ( "name" VARCHAR NOT NULL, "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, "visibility" VARCHAR NOT NULL, - "parent_path" TEXT + "parent_path" TEXT, + "requires_zed_cla" BOOLEAN NOT NULL DEFAULT FALSE ); CREATE INDEX "index_channels_on_parent_path" ON "channels" ("parent_path"); diff --git a/crates/collab/migrations/20240122224506_add_requires_zed_cla_column_to_channels.sql b/crates/collab/migrations/20240122224506_add_requires_zed_cla_column_to_channels.sql new file mode 100644 index 0000000000000000000000000000000000000000..a9248d294a2178b73986ab20cd06383d0397626b --- /dev/null +++ b/crates/collab/migrations/20240122224506_add_requires_zed_cla_column_to_channels.sql @@ -0,0 +1 @@ +ALTER TABLE "channels" ADD COLUMN "requires_zed_cla" BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index d0cac55df1bd7aaf57c5cae0b880fdf7b877023a..59d176b047b508d9748ba44e4aad700e60a1f58a 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -69,7 +69,7 @@ pub async fn validate_api_token(req: Request, next: Next) -> impl IntoR #[derive(Debug, Deserialize)] struct AuthenticatedUserParams { - github_user_id: i32, + github_user_id: Option, github_login: String, github_email: Option, } diff --git a/crates/collab/src/bin/seed.rs b/crates/collab/src/bin/seed.rs index bca2a7899a6bc2f49f760ca78ff78fb58ea4994b..ed24ccef75dce446eb54a431d1371139d67b7140 100644 --- a/crates/collab/src/bin/seed.rs +++ b/crates/collab/src/bin/seed.rs @@ -68,7 +68,7 @@ async fn main() { user_count += 1; db.get_or_create_user_by_github_account( &github_user.login, - github_user.id, + Some(github_user.id), github_user.email.as_deref(), ) .await diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index a920265b5703e55ef482a88c03facea95194265b..9a6a1e78f3cf03660697acdd463d803825e84e36 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -173,6 +173,14 @@ impl ChannelRole { Banned => false, } } + + pub fn requires_cla(&self) -> bool { + use ChannelRole::*; + match self { + Admin | Member => true, + Banned | Guest => false, + } + } } impl From for ChannelRole { diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 7ff9f00bc119c12a17987b2b7dff24e354286c33..c2428150fc4e53d5450330a58db01afad1f76c4c 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -67,6 +67,7 @@ impl Database { .as_ref() .map_or(String::new(), |parent| parent.path()), ), + requires_zed_cla: ActiveValue::NotSet, } .insert(&*tx) .await?; @@ -261,6 +262,22 @@ impl Database { .await } + #[cfg(test)] + pub async fn set_channel_requires_zed_cla( + &self, + channel_id: ChannelId, + requires_zed_cla: bool, + ) -> Result<()> { + self.transaction(move |tx| async move { + let channel = self.get_channel_internal(channel_id, &*tx).await?; + let mut model = channel.into_active_model(); + model.requires_zed_cla = ActiveValue::Set(requires_zed_cla); + model.update(&*tx).await?; + Ok(()) + }) + .await + } + /// Deletes the channel with the specified ID. pub async fn delete_channel( &self, diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs index 30d89cab7ba4d588bc81fe461c246aa5134f3a21..418e5dd1e14fa296dabfcf194a968f8ca45a330f 100644 --- a/crates/collab/src/db/queries/contributors.rs +++ b/crates/collab/src/db/queries/contributors.rs @@ -58,7 +58,7 @@ impl Database { pub async fn add_contributor( &self, github_login: &str, - github_user_id: i32, + github_user_id: Option, github_email: Option<&str>, ) -> Result<()> { self.transaction(|tx| async move { diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index 7434e2d20d006242ef572a5605c597fa13d71ade..c6aa5da125d5c74c0a86ea6b4d7cbed8fc1c07a0 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -1029,6 +1029,11 @@ impl Database { .await? .ok_or_else(|| anyhow!("only admins can set participant role"))?; + if role.requires_cla() { + self.check_user_has_signed_cla(user_id, room_id, &*tx) + .await?; + } + let result = room_participant::Entity::update_many() .filter( Condition::all() @@ -1050,6 +1055,45 @@ impl Database { .await } + async fn check_user_has_signed_cla( + &self, + user_id: UserId, + room_id: RoomId, + tx: &DatabaseTransaction, + ) -> Result<()> { + let channel = room::Entity::find_by_id(room_id) + .one(&*tx) + .await? + .ok_or_else(|| anyhow!("could not find room"))? + .find_related(channel::Entity) + .one(&*tx) + .await?; + + if let Some(channel) = channel { + let requires_zed_cla = channel.requires_zed_cla + || channel::Entity::find() + .filter( + channel::Column::Id + .is_in(channel.ancestors()) + .and(channel::Column::RequiresZedCla.eq(true)), + ) + .count(&*tx) + .await? + > 0; + if requires_zed_cla { + if contributor::Entity::find() + .filter(contributor::Column::UserId.eq(user_id)) + .one(&*tx) + .await? + .is_none() + { + Err(anyhow!("user has not signed the Zed CLA"))?; + } + } + } + Ok(()) + } + pub async fn connection_lost(&self, connection: ConnectionId) -> Result<()> { self.transaction(|tx| async move { self.room_connection_lost(connection, &*tx).await?; diff --git a/crates/collab/src/db/queries/users.rs b/crates/collab/src/db/queries/users.rs index 4249f06617709668c54e9a4b0b870f5b3f302af0..f0768a3a9ca6acf81df9c141115a4d4859830450 100644 --- a/crates/collab/src/db/queries/users.rs +++ b/crates/collab/src/db/queries/users.rs @@ -72,7 +72,7 @@ impl Database { pub async fn get_or_create_user_by_github_account( &self, github_login: &str, - github_user_id: i32, + github_user_id: Option, github_email: Option<&str>, ) -> Result { self.transaction(|tx| async move { @@ -90,39 +90,48 @@ impl Database { pub async fn get_or_create_user_by_github_account_tx( &self, github_login: &str, - github_user_id: i32, + github_user_id: Option, github_email: Option<&str>, tx: &DatabaseTransaction, ) -> Result { - if let Some(user_by_github_user_id) = user::Entity::find() - .filter(user::Column::GithubUserId.eq(github_user_id)) - .one(tx) - .await? - { - let mut user_by_github_user_id = user_by_github_user_id.into_active_model(); - user_by_github_user_id.github_login = ActiveValue::set(github_login.into()); - Ok(user_by_github_user_id.update(tx).await?) - } else if let Some(user_by_github_login) = user::Entity::find() - .filter(user::Column::GithubLogin.eq(github_login)) - .one(tx) - .await? - { - let mut user_by_github_login = user_by_github_login.into_active_model(); - user_by_github_login.github_user_id = ActiveValue::set(Some(github_user_id)); - Ok(user_by_github_login.update(tx).await?) + if let Some(github_user_id) = github_user_id { + if let Some(user_by_github_user_id) = user::Entity::find() + .filter(user::Column::GithubUserId.eq(github_user_id)) + .one(tx) + .await? + { + let mut user_by_github_user_id = user_by_github_user_id.into_active_model(); + user_by_github_user_id.github_login = ActiveValue::set(github_login.into()); + Ok(user_by_github_user_id.update(tx).await?) + } else if let Some(user_by_github_login) = user::Entity::find() + .filter(user::Column::GithubLogin.eq(github_login)) + .one(tx) + .await? + { + let mut user_by_github_login = user_by_github_login.into_active_model(); + user_by_github_login.github_user_id = ActiveValue::set(Some(github_user_id)); + Ok(user_by_github_login.update(tx).await?) + } else { + let user = user::Entity::insert(user::ActiveModel { + email_address: ActiveValue::set(github_email.map(|email| email.into())), + github_login: ActiveValue::set(github_login.into()), + github_user_id: ActiveValue::set(Some(github_user_id)), + admin: ActiveValue::set(false), + invite_count: ActiveValue::set(0), + invite_code: ActiveValue::set(None), + metrics_id: ActiveValue::set(Uuid::new_v4()), + ..Default::default() + }) + .exec_with_returning(&*tx) + .await?; + Ok(user) + } } else { - let user = user::Entity::insert(user::ActiveModel { - email_address: ActiveValue::set(github_email.map(|email| email.into())), - github_login: ActiveValue::set(github_login.into()), - github_user_id: ActiveValue::set(Some(github_user_id)), - admin: ActiveValue::set(false), - invite_count: ActiveValue::set(0), - invite_code: ActiveValue::set(None), - metrics_id: ActiveValue::set(Uuid::new_v4()), - ..Default::default() - }) - .exec_with_returning(&*tx) - .await?; + let user = user::Entity::find() + .filter(user::Column::GithubLogin.eq(github_login)) + .one(tx) + .await? + .ok_or_else(|| anyhow!("no such user {}", github_login))?; Ok(user) } } diff --git a/crates/collab/src/db/tables/channel.rs b/crates/collab/src/db/tables/channel.rs index e30ec9af61674a4ebab941239989aa87463b016b..a35913a705bd8bd63bcc0ec2bfe3cae7901a10d9 100644 --- a/crates/collab/src/db/tables/channel.rs +++ b/crates/collab/src/db/tables/channel.rs @@ -9,6 +9,7 @@ pub struct Model { pub name: String, pub visibility: ChannelVisibility, pub parent_path: String, + pub requires_zed_cla: bool, } impl Model { diff --git a/crates/collab/src/db/tests/contributor_tests.rs b/crates/collab/src/db/tests/contributor_tests.rs index 1985229f2f35a120a9db38ddf7f79bc74fad64b8..c826f0083a9a73db8af02b6570ac375ca702c22e 100644 --- a/crates/collab/src/db/tests/contributor_tests.rs +++ b/crates/collab/src/db/tests/contributor_tests.rs @@ -23,13 +23,13 @@ async fn test_contributors(db: &Arc) { assert_eq!(db.get_contributors().await.unwrap(), Vec::::new()); - db.add_contributor("user1", 1, None).await.unwrap(); + db.add_contributor("user1", Some(1), None).await.unwrap(); assert_eq!( db.get_contributors().await.unwrap(), vec!["user1".to_string()] ); - db.add_contributor("user2", 2, None).await.unwrap(); + db.add_contributor("user2", Some(2), None).await.unwrap(); assert_eq!( db.get_contributors().await.unwrap(), vec!["user1".to_string(), "user2".to_string()] diff --git a/crates/collab/src/db/tests/db_tests.rs b/crates/collab/src/db/tests/db_tests.rs index 7ae1a8a1a45b6c0ffe2b99d8106d4b6eb66aea93..adba6526c1b50b26930b5b1007a132bbf145bcfb 100644 --- a/crates/collab/src/db/tests/db_tests.rs +++ b/crates/collab/src/db/tests/db_tests.rs @@ -105,7 +105,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { .user_id; let user = db - .get_or_create_user_by_github_account("the-new-login2", 102, None) + .get_or_create_user_by_github_account("the-new-login2", Some(102), None) .await .unwrap(); assert_eq!(user.id, user_id2); @@ -113,7 +113,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { assert_eq!(user.github_user_id, Some(102)); let user = db - .get_or_create_user_by_github_account("login3", 103, Some("user3@example.com")) + .get_or_create_user_by_github_account("login3", Some(103), Some("user3@example.com")) .await .unwrap(); assert_eq!(&user.github_login, "login3"); diff --git a/crates/collab/src/tests/channel_guest_tests.rs b/crates/collab/src/tests/channel_guest_tests.rs index f3326cd6922b7482f6c9d953a5a57a89676b717d..26e9c56a4be806da69bbbbb895a1ed583c703a1f 100644 --- a/crates/collab/src/tests/channel_guest_tests.rs +++ b/crates/collab/src/tests/channel_guest_tests.rs @@ -1,4 +1,4 @@ -use crate::tests::TestServer; +use crate::{db::ChannelId, tests::TestServer}; use call::ActiveCall; use editor::Editor; use gpui::{BackgroundExecutor, TestAppContext}; @@ -159,3 +159,103 @@ async fn test_channel_guest_promotion(cx_a: &mut TestAppContext, cx_b: &mut Test .await .is_err()); } + +#[gpui::test] +async fn test_channel_requires_zed_cla(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { + let mut server = TestServer::start(cx_a.executor()).await; + + server + .app_state + .db + .get_or_create_user_by_github_account("user_b", Some(100), None) + .await + .unwrap(); + + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + let active_call_a = cx_a.read(ActiveCall::global); + let active_call_b = cx_b.read(ActiveCall::global); + + // Create a parent channel that requires the Zed CLA + let parent_channel_id = server + .make_channel("the-channel", None, (&client_a, cx_a), &mut []) + .await; + server + .app_state + .db + .set_channel_requires_zed_cla(ChannelId::from_proto(parent_channel_id), true) + .await + .unwrap(); + + // Create a public channel that is a child of the parent channel. + let channel_id = client_a + .channel_store() + .update(cx_a, |store, cx| { + store.create_channel("the-sub-channel", Some(parent_channel_id), cx) + }) + .await + .unwrap(); + client_a + .channel_store() + .update(cx_a, |store, cx| { + store.set_channel_visibility(channel_id, proto::ChannelVisibility::Public, cx) + }) + .await + .unwrap(); + + // Users A and B join the channel. B is a guest. + active_call_a + .update(cx_a, |call, cx| call.join_channel(channel_id, cx)) + .await + .unwrap(); + active_call_b + .update(cx_b, |call, cx| call.join_channel(channel_id, cx)) + .await + .unwrap(); + cx_a.run_until_parked(); + let room_b = cx_b + .read(ActiveCall::global) + .update(cx_b, |call, _| call.room().unwrap().clone()); + assert!(room_b.read_with(cx_b, |room, _| room.read_only())); + + // A tries to grant write access to B, but cannot because B has not + // yet signed the zed CLA. + active_call_a + .update(cx_a, |call, cx| { + call.room().unwrap().update(cx, |room, cx| { + room.set_participant_role( + client_b.user_id().unwrap(), + proto::ChannelRole::Member, + cx, + ) + }) + }) + .await + .unwrap_err(); + cx_a.run_until_parked(); + assert!(room_b.read_with(cx_b, |room, _| room.read_only())); + + // User B signs the zed CLA. + server + .app_state + .db + .add_contributor("user_b", Some(100), None) + .await + .unwrap(); + + // A can now grant write access to B. + active_call_a + .update(cx_a, |call, cx| { + call.room().unwrap().update(cx, |room, cx| { + room.set_participant_role( + client_b.user_id().unwrap(), + proto::ChannelRole::Member, + cx, + ) + }) + }) + .await + .unwrap(); + cx_a.run_until_parked(); + assert!(room_b.read_with(cx_b, |room, _| !room.read_only())); +} diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index 8efd9535b09bfb5e6243852a648c6cb1d5ad0450..9f25507c3dfbee12df4ca6deed0e795c0a2ff638 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -43,6 +43,7 @@ pub struct TestServer { pub app_state: Arc, pub test_live_kit_server: Arc, server: Arc, + next_github_user_id: i32, connection_killers: Arc>>>, forbid_connections: Arc, _test_db: TestDb, @@ -108,6 +109,7 @@ impl TestServer { server, connection_killers: Default::default(), forbid_connections: Default::default(), + next_github_user_id: 0, _test_db: test_db, test_live_kit_server: live_kit_server, } @@ -157,6 +159,8 @@ impl TestServer { { user.id } else { + let github_user_id = self.next_github_user_id; + self.next_github_user_id += 1; self.app_state .db .create_user( @@ -164,7 +168,7 @@ impl TestServer { false, NewUserParams { github_login: name.into(), - github_user_id: 0, + github_user_id, }, ) .await From ba5b969e100a73f38ab387eb337fca59fbbcffe0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 22 Jan 2024 17:30:55 -0800 Subject: [PATCH 15/88] collab 0.40.0 --- Cargo.lock | 2 +- crates/collab/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96f0e54f54c4ec234c38333fe7a3afcdd010976e..71e4e71c0223737043463fb78d6b9a4fab3df0b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1452,7 +1452,7 @@ dependencies = [ [[package]] name = "collab" -version = "0.39.0" +version = "0.40.0" dependencies = [ "anyhow", "async-trait", diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 3d5dfbc747b9d13deba405b19b2e648b31d7dbc8..bdcfd1d27bf2241ffe5543a5ad1c75ae1ef48c99 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] default-run = "collab" edition = "2021" name = "collab" -version = "0.39.0" +version = "0.40.0" publish = false [[bin]] From 1f94463ce2a23f0cf128fd62fbad5492da5430aa Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 21 Jan 2024 15:35:47 -0800 Subject: [PATCH 16/88] Switch Arc> to Rc>, a relic of the GPUI2 port. Make gpui pass clippy --- crates/gpui/src/app.rs | 17 +++++++++-------- crates/gpui/src/app/entity_map.rs | 5 ++--- crates/gpui/src/color.rs | 14 +++++--------- crates/gpui/src/element.rs | 4 +--- crates/gpui/src/elements/div.rs | 7 +++---- crates/gpui/src/elements/overlay.rs | 2 +- crates/gpui/src/elements/uniform_list.rs | 2 +- crates/gpui/src/geometry.rs | 4 ++-- crates/gpui/src/gpui.rs | 3 +++ crates/gpui/src/interactive.rs | 2 +- crates/gpui/src/key_dispatch.rs | 15 ++++++--------- crates/gpui/src/{keymap => }/keymap.rs | 10 +++++++++- crates/gpui/src/keymap/context.rs | 4 ++-- crates/gpui/src/keymap/matcher.rs | 12 ++++++------ crates/gpui/src/keymap/mod.rs | 9 --------- crates/gpui/src/subscription.rs | 1 - crates/gpui/src/taffy.rs | 14 ++++---------- crates/gpui/src/text_system.rs | 2 +- crates/gpui/src/text_system/line.rs | 2 ++ crates/gpui/src/text_system/line_layout.rs | 1 + crates/gpui/src/window.rs | 2 +- crates/media/src/media.rs | 15 +++++++++++++++ 22 files changed, 75 insertions(+), 72 deletions(-) rename crates/gpui/src/{keymap => }/keymap.rs (97%) delete mode 100644 crates/gpui/src/keymap/mod.rs diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 7b349d92568e72b9b4929655e93dd1683636964e..dfecffb80a26461cc16d40489213988494229672 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -25,13 +25,12 @@ use crate::{ use anyhow::{anyhow, Result}; use collections::{FxHashMap, FxHashSet, VecDeque}; use futures::{channel::oneshot, future::LocalBoxFuture, Future}; -use parking_lot::Mutex; + use slotmap::SlotMap; use std::{ any::{type_name, TypeId}, cell::{Ref, RefCell, RefMut}, marker::PhantomData, - mem, ops::{Deref, DerefMut}, path::{Path, PathBuf}, rc::{Rc, Weak}, @@ -109,6 +108,7 @@ pub struct App(Rc); /// configured, you'll start the app with `App::run`. impl App { /// Builds an app with the given asset source. + #[allow(clippy::new_without_default)] pub fn new() -> Self { Self(AppContext::new( current_platform(), @@ -224,7 +224,7 @@ pub struct AppContext { pub(crate) entities: EntityMap, pub(crate) new_view_observers: SubscriberSet, pub(crate) windows: SlotMap>, - pub(crate) keymap: Arc>, + pub(crate) keymap: Rc>, pub(crate) global_action_listeners: FxHashMap>>, pending_effects: VecDeque, @@ -242,6 +242,7 @@ pub struct AppContext { } impl AppContext { + #[allow(clippy::new_ret_no_self)] pub(crate) fn new( platform: Rc, asset_source: Arc, @@ -285,7 +286,7 @@ impl AppContext { entities, new_view_observers: SubscriberSet::new(), windows: SlotMap::with_key(), - keymap: Arc::new(Mutex::new(Keymap::default())), + keymap: Rc::new(RefCell::new(Keymap::default())), global_action_listeners: FxHashMap::default(), pending_effects: VecDeque::new(), pending_notifications: FxHashSet::default(), @@ -763,7 +764,7 @@ impl AppContext { /// so it can be held across `await` points. pub fn to_async(&self) -> AsyncAppContext { AsyncAppContext { - app: unsafe { mem::transmute(self.this.clone()) }, + app: self.this.clone(), background_executor: self.background_executor.clone(), foreground_executor: self.foreground_executor.clone(), } @@ -996,13 +997,13 @@ impl AppContext { /// Register key bindings. pub fn bind_keys(&mut self, bindings: impl IntoIterator) { - self.keymap.lock().add_bindings(bindings); + self.keymap.borrow_mut().add_bindings(bindings); self.pending_effects.push_back(Effect::Refresh); } /// Clear all key bindings in the app. pub fn clear_key_bindings(&mut self) { - self.keymap.lock().clear(); + self.keymap.borrow_mut().clear(); self.pending_effects.push_back(Effect::Refresh); } @@ -1106,7 +1107,7 @@ impl AppContext { /// Sets the menu bar for this application. This will replace any existing menu bar. pub fn set_menus(&mut self, menus: Vec) { - self.platform.set_menus(menus, &self.keymap.lock()); + self.platform.set_menus(menus, &self.keymap.borrow()); } /// Dispatch an action to the currently active window or global action handler diff --git a/crates/gpui/src/app/entity_map.rs b/crates/gpui/src/app/entity_map.rs index db5d844d17c173bbc0541d49aaf5f67e70f139a0..6383cdad7b5efba1859f34fc4ce16c11520d4576 100644 --- a/crates/gpui/src/app/entity_map.rs +++ b/crates/gpui/src/app/entity_map.rs @@ -242,7 +242,7 @@ impl Clone for AnyModel { assert_ne!(prev_count, 0, "Detected over-release of a model."); } - let this = Self { + Self { entity_id: self.entity_id, entity_type: self.entity_type, entity_map: self.entity_map.clone(), @@ -254,8 +254,7 @@ impl Clone for AnyModel { .write() .leak_detector .handle_created(self.entity_id), - }; - this + } } } diff --git a/crates/gpui/src/color.rs b/crates/gpui/src/color.rs index e76c31d6f15db5a7690aab3bafd0b950f79e2824..dadc3d5ad301ba3cd2145215bb7909701fd9ed27 100644 --- a/crates/gpui/src/color.rs +++ b/crates/gpui/src/color.rs @@ -203,20 +203,16 @@ impl PartialEq for Hsla { impl PartialOrd for Hsla { fn partial_cmp(&self, other: &Self) -> Option { - // SAFETY: The total ordering relies on this always being Some() - Some( - self.h - .total_cmp(&other.h) - .then(self.s.total_cmp(&other.s)) - .then(self.l.total_cmp(&other.l).then(self.a.total_cmp(&other.a))), - ) + Some(self.cmp(other)) } } impl Ord for Hsla { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - // SAFETY: The partial comparison is a total comparison - unsafe { self.partial_cmp(other).unwrap_unchecked() } + self.h + .total_cmp(&other.h) + .then(self.s.total_cmp(&other.s)) + .then(self.l.total_cmp(&other.l).then(self.a.total_cmp(&other.a))) } } diff --git a/crates/gpui/src/element.rs b/crates/gpui/src/element.rs index cd5e6ea9dcd2af407e1de2cfa2f6f25e3e6c392c..4dbc7be652d3acf73bc90e3c709039fb79223074 100644 --- a/crates/gpui/src/element.rs +++ b/crates/gpui/src/element.rs @@ -133,9 +133,7 @@ pub trait Render: 'static + Sized { } impl Render for () { - fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement { - () - } + fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement {} } /// You can derive [`IntoElement`] on any type that implements this trait. diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index b1d936546b91f0387bfacf907d309a78d79d4a44..7ccdf8c2bf04891d9084b2170dc926874143a563 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -1008,10 +1008,9 @@ pub(crate) type ActionListener = Box Div { #[cfg(debug_assertions)] - let interactivity = { - let mut interactivity = Interactivity::default(); - interactivity.location = Some(*core::panic::Location::caller()); - interactivity + let interactivity = Interactivity { + location: Some(*core::panic::Location::caller()), + ..Default::default() }; #[cfg(not(debug_assertions))] diff --git a/crates/gpui/src/elements/overlay.rs b/crates/gpui/src/elements/overlay.rs index 61c34bd9385f6580feb3930dd3b3ea4b1494e919..9db75b75ba0d8138d75fd4a9aa90071e15689a10 100644 --- a/crates/gpui/src/elements/overlay.rs +++ b/crates/gpui/src/elements/overlay.rs @@ -222,7 +222,7 @@ impl OverlayPositionMode { ) -> (Point, Bounds) { match self { OverlayPositionMode::Window => { - let anchor_position = anchor_position.unwrap_or_else(|| bounds.origin); + let anchor_position = anchor_position.unwrap_or(bounds.origin); let bounds = anchor_corner.get_bounds(anchor_position, size); (anchor_position, bounds) } diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 108e669f7550a2e6387641aaa4c1dae6e59f6313..ce32b993a5592732f763712b84fa33f24e2738f4 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -181,7 +181,7 @@ impl Element for UniformList { let shared_scroll_offset = element_state .interactive .scroll_offset - .get_or_insert_with(|| Rc::default()) + .get_or_insert_with(Rc::default) .clone(); let item_height = self.measure_item(Some(padded_bounds.size.width), cx).height; diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index d986f26be4475a72b0da8b75de8b3847e3fce588..0a22611fac8a2bdc38087710e4509cf57feb9e57 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -2020,13 +2020,13 @@ impl Eq for Pixels {} impl PartialOrd for Pixels { fn partial_cmp(&self, other: &Self) -> Option { - self.0.partial_cmp(&other.0) + Some(self.cmp(other)) } } impl Ord for Pixels { fn cmp(&self, other: &Self) -> cmp::Ordering { - self.partial_cmp(other).unwrap() + self.0.total_cmp(&other.0) } } diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 929e9ebfb4f262088e90e7ab5b99477a9933d998..a64d043ab87e84d1fdf2f9812db205616503ead5 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -26,6 +26,9 @@ //! TODO!(docs): Wrap up with a conclusion and links to other places? Zed / GPUI website? //! Discord for chatting about it? Other tutorials or references? +// #![deny(missing_docs)] +#![allow(clippy::type_complexity)] + #[macro_use] mod action; mod app; diff --git a/crates/gpui/src/interactive.rs b/crates/gpui/src/interactive.rs index 4faa92ce043ae32e6e795b296daeb9ad281475f1..1bc32717c4b7702ca37ee413092d13e12fc6114f 100644 --- a/crates/gpui/src/interactive.rs +++ b/crates/gpui/src/interactive.rs @@ -343,7 +343,7 @@ impl ExternalPaths { impl Render for ExternalPaths { fn render(&mut self, _: &mut ViewContext) -> impl IntoElement { - () // Intentionally left empty because the platform will render icons for the dragged files + // Intentionally left empty because the platform will render icons for the dragged files } } diff --git a/crates/gpui/src/key_dispatch.rs b/crates/gpui/src/key_dispatch.rs index dd5a7ab84e25e267012de5e9dfb216404b8dfa7d..b4de09e36c58e6b32bacac0e0b90d160f3de0195 100644 --- a/crates/gpui/src/key_dispatch.rs +++ b/crates/gpui/src/key_dispatch.rs @@ -54,13 +54,12 @@ use crate::{ KeyContext, Keymap, KeymatchResult, Keystroke, KeystrokeMatcher, WindowContext, }; use collections::FxHashMap; -use parking_lot::Mutex; use smallvec::{smallvec, SmallVec}; use std::{ any::{Any, TypeId}, + cell::RefCell, mem, rc::Rc, - sync::Arc, }; #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] @@ -73,7 +72,7 @@ pub(crate) struct DispatchTree { focusable_node_ids: FxHashMap, view_node_ids: FxHashMap, keystroke_matchers: FxHashMap, KeystrokeMatcher>, - keymap: Arc>, + keymap: Rc>, action_registry: Rc, } @@ -96,7 +95,7 @@ pub(crate) struct DispatchActionListener { } impl DispatchTree { - pub fn new(keymap: Arc>, action_registry: Rc) -> Self { + pub fn new(keymap: Rc>, action_registry: Rc) -> Self { Self { node_stack: Vec::new(), context_stack: Vec::new(), @@ -307,7 +306,7 @@ impl DispatchTree { action: &dyn Action, context_stack: &Vec, ) -> Vec { - let keymap = self.keymap.lock(); + let keymap = self.keymap.borrow(); keymap .bindings_for_action(action) .filter(|binding| { @@ -440,9 +439,7 @@ impl DispatchTree { #[cfg(test)] mod tests { - use std::{rc::Rc, sync::Arc}; - - use parking_lot::Mutex; + use std::{cell::RefCell, rc::Rc}; use crate::{Action, ActionRegistry, DispatchTree, KeyBinding, KeyContext, Keymap}; @@ -496,7 +493,7 @@ mod tests { registry.load_action::(); - let keymap = Arc::new(Mutex::new(keymap)); + let keymap = Rc::new(RefCell::new(keymap)); let tree = DispatchTree::new(keymap, Rc::new(registry)); diff --git a/crates/gpui/src/keymap/keymap.rs b/crates/gpui/src/keymap.rs similarity index 97% rename from crates/gpui/src/keymap/keymap.rs rename to crates/gpui/src/keymap.rs index 2eefbb841ee5be0b499e7b32aaf28aa7da36a072..45e0ebbe951fe4a260cf869787e24322a47b1bd0 100644 --- a/crates/gpui/src/keymap/keymap.rs +++ b/crates/gpui/src/keymap.rs @@ -1,4 +1,12 @@ -use crate::{Action, KeyBinding, KeyBindingContextPredicate, KeyContext, Keystroke, NoAction}; +mod binding; +mod context; +mod matcher; + +pub use binding::*; +pub use context::*; +pub(crate) use matcher::*; + +use crate::{Action, Keystroke, NoAction}; use collections::HashSet; use smallvec::SmallVec; use std::{ diff --git a/crates/gpui/src/keymap/context.rs b/crates/gpui/src/keymap/context.rs index 05ef67ba2bc9d260215fc0df89a8a6e3039450f5..6c22fa9fd69bfacf650e549d786ebfc0cb83de21 100644 --- a/crates/gpui/src/keymap/context.rs +++ b/crates/gpui/src/keymap/context.rs @@ -267,8 +267,8 @@ impl KeyBindingContextPredicate { '(' => { source = skip_whitespace(&source[1..]); let (predicate, rest) = Self::parse_expr(source, 0)?; - if rest.starts_with(')') { - source = skip_whitespace(&rest[1..]); + if let Some(stripped) = rest.strip_prefix(')') { + source = skip_whitespace(stripped); Ok((predicate, source)) } else { Err(anyhow!("expected a ')'")) diff --git a/crates/gpui/src/keymap/matcher.rs b/crates/gpui/src/keymap/matcher.rs index 09ba281a0d7ebc1e032100eb9463f32767afa403..88a8826e653dc6418313df723f2748baf588bb66 100644 --- a/crates/gpui/src/keymap/matcher.rs +++ b/crates/gpui/src/keymap/matcher.rs @@ -1,11 +1,10 @@ use crate::{KeyBinding, KeyContext, Keymap, KeymapVersion, Keystroke}; -use parking_lot::Mutex; use smallvec::SmallVec; -use std::sync::Arc; +use std::{cell::RefCell, rc::Rc}; pub(crate) struct KeystrokeMatcher { pending_keystrokes: Vec, - keymap: Arc>, + keymap: Rc>, keymap_version: KeymapVersion, } @@ -15,8 +14,8 @@ pub struct KeymatchResult { } impl KeystrokeMatcher { - pub fn new(keymap: Arc>) -> Self { - let keymap_version = keymap.lock().version(); + pub fn new(keymap: Rc>) -> Self { + let keymap_version = keymap.borrow().version(); Self { pending_keystrokes: Vec::new(), keymap_version, @@ -42,7 +41,8 @@ impl KeystrokeMatcher { keystroke: &Keystroke, context_stack: &[KeyContext], ) -> KeymatchResult { - let keymap = self.keymap.lock(); + let keymap = self.keymap.borrow(); + // Clear pending keystrokes if the keymap has changed since the last matched keystroke. if keymap.version() != self.keymap_version { self.keymap_version = keymap.version(); diff --git a/crates/gpui/src/keymap/mod.rs b/crates/gpui/src/keymap/mod.rs deleted file mode 100644 index 6f1a018322b1a7f55349af70729860b9d85bde60..0000000000000000000000000000000000000000 --- a/crates/gpui/src/keymap/mod.rs +++ /dev/null @@ -1,9 +0,0 @@ -mod binding; -mod context; -mod keymap; -mod matcher; - -pub use binding::*; -pub use context::*; -pub use keymap::*; -pub(crate) use matcher::*; diff --git a/crates/gpui/src/subscription.rs b/crates/gpui/src/subscription.rs index 9dca2e3a4841cc6c4ccd85c16700d08201218bd2..c75d10ee6289ed7c7e02436ca4aed54b78675bda 100644 --- a/crates/gpui/src/subscription.rs +++ b/crates/gpui/src/subscription.rs @@ -41,7 +41,6 @@ where /// are inert, meaning that they won't be listed when calling `[SubscriberSet::remove]` or `[SubscriberSet::retain]`. /// This method returns a tuple of a [`Subscription`] and an `impl FnOnce`, and you can use the latter /// to activate the [`Subscription`]. - #[must_use] pub fn insert( &self, emitter_key: EmitterKey, diff --git a/crates/gpui/src/taffy.rs b/crates/gpui/src/taffy.rs index ea7a4575cc3a22ae820e37f7a26e1f4f1a78a2d7..0797c8f3b497b561897ee68fd3216499824b59db 100644 --- a/crates/gpui/src/taffy.rs +++ b/crates/gpui/src/taffy.rs @@ -12,22 +12,16 @@ use taffy::{ Taffy, }; +type NodeMeasureFn = + Box>, Size, &mut WindowContext) -> Size>; + pub struct TaffyLayoutEngine { taffy: Taffy, styles: FxHashMap, children_to_parents: FxHashMap, absolute_layout_bounds: FxHashMap>, computed_layouts: FxHashSet, - nodes_to_measure: FxHashMap< - LayoutId, - Box< - dyn FnMut( - Size>, - Size, - &mut WindowContext, - ) -> Size, - >, - >, + nodes_to_measure: FxHashMap, } static EXPECT_MESSAGE: &str = "we should avoid taffy layout errors by construction if possible"; diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index cadc000f9a203f879a83e529c325061f7db0eeba..1d097ca1eed23dd316476d293b0c0944b346567a 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -71,7 +71,7 @@ impl TextSystem { .all_font_names() .into_iter() .collect(); - names.extend(self.platform_text_system.all_font_families().into_iter()); + names.extend(self.platform_text_system.all_font_families()); names.extend( self.fallback_font_stack .iter() diff --git a/crates/gpui/src/text_system/line.rs b/crates/gpui/src/text_system/line.rs index 8013f5a1e05e3e61276d89db4d3cb76c26b7ba80..d4f23c9539f16840ab224b924f96348141ada902 100644 --- a/crates/gpui/src/text_system/line.rs +++ b/crates/gpui/src/text_system/line.rs @@ -25,6 +25,7 @@ pub struct ShapedLine { impl ShapedLine { /// The length of the line in utf-8 bytes. + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.layout.len } @@ -58,6 +59,7 @@ pub struct WrappedLine { } impl WrappedLine { + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.layout.len() } diff --git a/crates/gpui/src/text_system/line_layout.rs b/crates/gpui/src/text_system/line_layout.rs index 6c466f9680e8ec8ff75812bcdf15a172c9ff6258..166fefd26a55822bd6ff4475a7f839c20d849d2c 100644 --- a/crates/gpui/src/text_system/line_layout.rs +++ b/crates/gpui/src/text_system/line_layout.rs @@ -162,6 +162,7 @@ pub struct WrapBoundary { } impl WrappedLineLayout { + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.unwrapped_layout.len } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 5796d139f91f0ca822dec66efb6b048fbb1c6af4..5ffe753dc1d179843dfd7a14ddf234d13d366861 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -2560,7 +2560,7 @@ impl Display for ElementId { ElementId::View(entity_id) => write!(f, "view-{}", entity_id)?, ElementId::Integer(ix) => write!(f, "{}", ix)?, ElementId::Name(name) => write!(f, "{}", name)?, - ElementId::FocusHandle(__) => write!(f, "FocusHandle")?, + ElementId::FocusHandle(_) => write!(f, "FocusHandle")?, ElementId::NamedInteger(s, i) => write!(f, "{}-{}", s, i)?, } diff --git a/crates/media/src/media.rs b/crates/media/src/media.rs index 650af06c37dbbc325bc825c06e375fef8adfe6a5..269244dbf9d15f4de4438fe180f4f1f8cb3cd0c5 100644 --- a/crates/media/src/media.rs +++ b/crates/media/src/media.rs @@ -108,6 +108,9 @@ pub mod core_video { impl_CFTypeDescription!(CVMetalTextureCache); impl CVMetalTextureCache { + /// # Safety + /// + /// metal_device must be valid according to CVMetalTextureCacheCreate pub unsafe fn new(metal_device: *mut MTLDevice) -> Result { let mut this = ptr::null(); let result = CVMetalTextureCacheCreate( @@ -124,6 +127,9 @@ pub mod core_video { } } + /// # Safety + /// + /// The arguments to this function must be valid according to CVMetalTextureCacheCreateTextureFromImage pub unsafe fn create_texture_from_image( &self, source: CVImageBufferRef, @@ -434,6 +440,12 @@ pub mod video_toolbox { impl_CFTypeDescription!(VTCompressionSession); impl VTCompressionSession { + /// Create a new compression session. + /// + /// # Safety + /// + /// The callback must be a valid function pointer. and the callback_data must be valid + /// in whatever terms that callback expects. pub unsafe fn new( width: usize, height: usize, @@ -465,6 +477,9 @@ pub mod video_toolbox { } } + /// # Safety + /// + /// The arguments to this function must be valid according to VTCompressionSessionEncodeFrame pub unsafe fn encode_frame( &self, buffer: CVImageBufferRef, From 1902df931608eb351b34d48175437c557bc16a03 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 21 Jan 2024 15:51:33 -0800 Subject: [PATCH 17/88] WIP: Start geometry crate --- crates/gpui/src/geometry.rs | 8 ++++++++ crates/gpui/src/gpui.rs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index 0a22611fac8a2bdc38087710e4509cf57feb9e57..ef71f94ef664d381fef8fa54ba72718fdcb5cf99 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -1,3 +1,7 @@ +//! The GPUI geometry module is a collection of types and traits that +//! can be used to describe common units, concepts, and the relationships +//! between them. + use core::fmt::Debug; use derive_more::{Add, AddAssign, Div, DivAssign, Mul, Neg, Sub, SubAssign}; use refineable::Refineable; @@ -8,13 +12,17 @@ use std::{ ops::{Add, Div, Mul, MulAssign, Sub}, }; +/// An axis along which a measurement can be made. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum Axis { + /// The y axis, or up and down Vertical, + /// The x axis, or left and right Horizontal, } impl Axis { + /// Swap this axis to the opposite axis. pub fn invert(&self) -> Self { match self { Axis::Vertical => Axis::Horizontal, diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index a64d043ab87e84d1fdf2f9812db205616503ead5..8ecfd8ad6d834ac8c6577e2f724e22719e15e36e 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -26,7 +26,7 @@ //! TODO!(docs): Wrap up with a conclusion and links to other places? Zed / GPUI website? //! Discord for chatting about it? Other tutorials or references? -// #![deny(missing_docs)] +#![deny(missing_docs)] #![allow(clippy::type_complexity)] #[macro_use] From 0c3fb449f044d550b871abfa3f568343ddf38cc5 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 22 Jan 2024 08:11:21 -0800 Subject: [PATCH 18/88] Document geometry --- crates/gpui/src/geometry.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index ef71f94ef664d381fef8fa54ba72718fdcb5cf99..dd826b68f541c98c88d364156dafc641bdbc8afa 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -31,11 +31,15 @@ impl Axis { } } +/// A trait for accessing the given unit along a certain axis. pub trait Along { + /// The unit associated with this type type Unit; + /// Returns the unit along the given axis. fn along(&self, axis: Axis) -> Self::Unit; + /// Applies the given function to the unit along the given axis and returns a new value. fn apply_along(&self, axis: Axis, f: impl FnOnce(Self::Unit) -> Self::Unit) -> Self; } @@ -55,7 +59,9 @@ pub trait Along { #[refineable(Debug)] #[repr(C)] pub struct Point { + /// The x coordinate of the point. pub x: T, + /// The y coordinate of the point. pub y: T, } @@ -342,7 +348,9 @@ impl Clone for Point { #[refineable(Debug)] #[repr(C)] pub struct Size { + /// The width component of the size. pub width: T, + /// The height component of the size. pub height: T, } @@ -648,7 +656,9 @@ impl Size { #[refineable(Debug)] #[repr(C)] pub struct Bounds { + /// The origin point of this area. pub origin: Point, + /// The size of the rectangle. pub size: Size, } @@ -1200,9 +1210,13 @@ impl Copy for Bounds {} #[refineable(Debug)] #[repr(C)] pub struct Edges { + /// The size of the top edge. pub top: T, + /// The size of the right edge. pub right: T, + /// The size of the bottom edge. pub bottom: T, + /// The size of the left edge. pub left: T, } @@ -1608,9 +1622,13 @@ impl From for Edges { #[refineable(Debug)] #[repr(C)] pub struct Corners { + /// The value associated with the top left corner. pub top_left: T, + /// The value associated with the top right corner. pub top_right: T, + /// The value associated with the bottom right corner. pub bottom_right: T, + /// The value associated with the bottom left corner. pub bottom_left: T, } From a99d5b87e8ed0900b433f8b0c28e092c5c62bb0c Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 22 Jan 2024 09:35:01 -0800 Subject: [PATCH 19/88] WIP: text_system --- crates/gpui/src/platform/mac/text_system.rs | 14 +-- crates/gpui/src/text_system.rs | 90 ++++++++++++++----- crates/gpui/src/text_system/font_features.rs | 7 +- crates/gpui/src/window/element_cx.rs | 24 ++++- .../src/derive_refineable.rs | 7 +- 5 files changed, 106 insertions(+), 36 deletions(-) diff --git a/crates/gpui/src/platform/mac/text_system.rs b/crates/gpui/src/platform/mac/text_system.rs index 21fe3c70a8a8787056fd3409c837af76a70427cf..138a465775f83e4297417ca3ae6f365dc93873bc 100644 --- a/crates/gpui/src/platform/mac/text_system.rs +++ b/crates/gpui/src/platform/mac/text_system.rs @@ -143,7 +143,7 @@ impl PlatformTextSystem for MacTextSystem { fn typographic_bounds(&self, font_id: FontId, glyph_id: GlyphId) -> Result> { Ok(self.0.read().fonts[font_id.0] - .typographic_bounds(glyph_id.into())? + .typographic_bounds(glyph_id.0)? .into()) } @@ -221,11 +221,13 @@ impl MacTextSystemState { } fn advance(&self, font_id: FontId, glyph_id: GlyphId) -> Result> { - Ok(self.fonts[font_id.0].advance(glyph_id.into())?.into()) + Ok(self.fonts[font_id.0].advance(glyph_id.0)?.into()) } fn glyph_for_char(&self, font_id: FontId, ch: char) -> Option { - self.fonts[font_id.0].glyph_for_char(ch).map(Into::into) + self.fonts[font_id.0] + .glyph_for_char(ch) + .map(|glyph_id| GlyphId(glyph_id)) } fn id_for_native_font(&mut self, requested_font: CTFont) -> FontId { @@ -259,7 +261,7 @@ impl MacTextSystemState { let scale = Transform2F::from_scale(params.scale_factor); Ok(font .raster_bounds( - params.glyph_id.into(), + params.glyph_id.0, params.font_size.into(), scale, HintingOptions::None, @@ -334,7 +336,7 @@ impl MacTextSystemState { .native_font() .clone_with_font_size(f32::from(params.font_size) as CGFloat) .draw_glyphs( - &[u32::from(params.glyph_id) as CGGlyph], + &[params.glyph_id.0 as CGGlyph], &[CGPoint::new( (subpixel_shift.x / params.scale_factor) as CGFloat, (subpixel_shift.y / params.scale_factor) as CGFloat, @@ -419,7 +421,7 @@ impl MacTextSystemState { let glyph_utf16_ix = usize::try_from(*glyph_utf16_ix).unwrap(); ix_converter.advance_to_utf16_ix(glyph_utf16_ix); glyphs.push(ShapedGlyph { - id: (*glyph_id).into(), + id: GlyphId(*glyph_id as u32), position: point(position.x as f32, position.y as f32).map(px), index: ix_converter.utf8_ix, is_emoji: self.is_emoji(font_id), diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 1d097ca1eed23dd316476d293b0c0944b346567a..9662247e6f248bc329a195c5abb44373d8151fdd 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -26,15 +26,18 @@ use std::{ sync::Arc, }; +/// An opaque identifier for a specific font. #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] #[repr(C)] pub struct FontId(pub usize); +/// An opaque identifier for a specific font family. #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] pub struct FontFamilyId(pub usize); pub(crate) const SUBPIXEL_VARIANTS: u8 = 4; +/// The GPUI text layout and rendering sub system. pub struct TextSystem { line_layout_cache: Arc, platform_text_system: Arc, @@ -65,6 +68,7 @@ impl TextSystem { } } + /// Get a list of all available font names from the operating system. pub fn all_font_names(&self) -> Vec { let mut names: BTreeSet<_> = self .platform_text_system @@ -79,10 +83,13 @@ impl TextSystem { ); names.into_iter().collect() } + + /// Add a font's data to the text system. pub fn add_fonts(&self, fonts: &[Arc>]) -> Result<()> { self.platform_text_system.add_fonts(fonts) } + /// Get the FontId for the configure font family and style. pub fn font_id(&self, font: &Font) -> Result { let font_id = self.font_ids_by_font.read().get(font).copied(); if let Some(font_id) = font_id { @@ -120,10 +127,14 @@ impl TextSystem { ); } + /// Get the bounding box for the given font and font size. + /// A font's bounding box is the smallest rectangle that could enclose all glyphs + /// in the font. superimposed over one another. pub fn bounding_box(&self, font_id: FontId, font_size: Pixels) -> Bounds { self.read_metrics(font_id, |metrics| metrics.bounding_box(font_size)) } + /// Get the typographic bounds for the given character, in the given font and size. pub fn typographic_bounds( &self, font_id: FontId, @@ -142,6 +153,7 @@ impl TextSystem { })) } + /// Get the advance width for the given character, in the given font and size. pub fn advance(&self, font_id: FontId, font_size: Pixels, ch: char) -> Result> { let glyph_id = self .platform_text_system @@ -153,26 +165,35 @@ impl TextSystem { Ok(result * font_size) } + /// Get the number of font size units per 'em square', + /// Per MDN: "an abstract square whose height is the intended distance between + /// lines of type in the same type size" pub fn units_per_em(&self, font_id: FontId) -> u32 { self.read_metrics(font_id, |metrics| metrics.units_per_em) } + /// Get the height of a captial letter in the given font and size. pub fn cap_height(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.cap_height(font_size)) } + /// Get the height of the x character in the given font and size. pub fn x_height(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.x_height(font_size)) } + /// Get the recommended distance from the baseline for the given font pub fn ascent(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.ascent(font_size)) } + /// Get the recommended distance below the baseline for the given font, + /// in single spaced text. pub fn descent(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.descent(font_size)) } + /// Get the recommended baseline offset for the given font and line height. pub fn baseline_offset( &self, font_id: FontId, @@ -199,10 +220,14 @@ impl TextSystem { } } - pub fn with_view(&self, view_id: EntityId, f: impl FnOnce() -> R) -> R { + pub(crate) fn with_view(&self, view_id: EntityId, f: impl FnOnce() -> R) -> R { self.line_layout_cache.with_view(view_id, f) } + /// Layout the given line of text, at the given font_size. + /// Subsets of the line can be styled independently with the `runs` parameter. + /// Generally, you should prefer to use `TextLayout::shape_line` instead, which + /// can be painted directly. pub fn layout_line( &self, text: &str, @@ -234,6 +259,12 @@ impl TextSystem { Ok(layout) } + /// Shape the given line, at the given font_size, for painting to the screen. + /// Subsets of the line can be styled independently with the `runs` parameter. + /// + /// Note that this method can only shape a single line of text. It will panic + /// if the text contains newlines. If you need to shape multiple lines of text, + /// use `TextLayout::shape_text` instead. pub fn shape_line( &self, text: SharedString, @@ -273,6 +304,9 @@ impl TextSystem { }) } + /// Shape a multi line string of text, at the given font_size, for painting to the screen. + /// Subsets of the text can be styled independently with the `runs` parameter. + /// If `wrap_width` is provided, the line breaks will be adjusted to fit within the given width. pub fn shape_text( &self, text: SharedString, @@ -381,6 +415,7 @@ impl TextSystem { self.line_layout_cache.finish_frame(reused_views) } + /// Returns a handle to a line wrapper, for the given font and font size. pub fn line_wrapper(self: &Arc, font: Font, font_size: Pixels) -> LineWrapperHandle { let lock = &mut self.wrapper_pool.lock(); let font_id = self.resolve_font(&font); @@ -397,7 +432,8 @@ impl TextSystem { } } - pub fn raster_bounds(&self, params: &RenderGlyphParams) -> Result> { + /// Get the rasterized size and location of a specific, rendered glyph. + pub(crate) fn raster_bounds(&self, params: &RenderGlyphParams) -> Result> { let raster_bounds = self.raster_bounds.upgradable_read(); if let Some(bounds) = raster_bounds.get(params) { Ok(*bounds) @@ -409,7 +445,7 @@ impl TextSystem { } } - pub fn rasterize_glyph( + pub(crate) fn rasterize_glyph( &self, params: &RenderGlyphParams, ) -> Result<(Size, Vec)> { @@ -425,6 +461,7 @@ struct FontIdWithSize { font_size: Pixels, } +/// A handle into the text system, which can be used to compute the wrapped layout of text pub struct LineWrapperHandle { wrapper: Option, text_system: Arc, @@ -517,40 +554,28 @@ impl Display for FontStyle { } } +/// A styled run of text, for use in [`TextLayout`]. #[derive(Clone, Debug, PartialEq, Eq)] pub struct TextRun { - // number of utf8 bytes + /// A number of utf8 bytes pub len: usize, + /// The font to use for this run. pub font: Font, + /// The color pub color: Hsla, + /// The background color (if any) pub background_color: Option, + /// The underline style (if any) pub underline: Option, } +/// An identifier for a specific glyph, as returned by [`TextSystem::layout_line`]. #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] #[repr(C)] -pub struct GlyphId(u32); - -impl From for u32 { - fn from(value: GlyphId) -> Self { - value.0 - } -} - -impl From for GlyphId { - fn from(num: u16) -> Self { - GlyphId(num as u32) - } -} - -impl From for GlyphId { - fn from(num: u32) -> Self { - GlyphId(num) - } -} +pub struct GlyphId(pub(crate) u32); #[derive(Clone, Debug, PartialEq)] -pub struct RenderGlyphParams { +pub(crate) struct RenderGlyphParams { pub(crate) font_id: FontId, pub(crate) glyph_id: GlyphId, pub(crate) font_size: Pixels, @@ -571,6 +596,7 @@ impl Hash for RenderGlyphParams { } } +/// The parameters for rendering an emoji glyph. #[derive(Clone, Debug, PartialEq)] pub struct RenderEmojiParams { pub(crate) font_id: FontId, @@ -590,14 +616,23 @@ impl Hash for RenderEmojiParams { } } +/// The configuration details for identifying a specific font. #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub struct Font { + /// The font family name. pub family: SharedString, + + /// The font features to use. pub features: FontFeatures, + + /// The font weight. pub weight: FontWeight, + + /// The font style. pub style: FontStyle, } +/// Get a [`Font`] for a given name. pub fn font(family: impl Into) -> Font { Font { family: family.into(), @@ -608,10 +643,17 @@ pub fn font(family: impl Into) -> Font { } impl Font { + /// Set this Font to be bold pub fn bold(mut self) -> Self { self.weight = FontWeight::BOLD; self } + + /// Set this Font to be italic + pub fn italic(mut self) -> Self { + self.style = FontStyle::Italic; + self + } } /// A struct for storing font metrics. diff --git a/crates/gpui/src/text_system/font_features.rs b/crates/gpui/src/text_system/font_features.rs index ce93fd5bc93fe59c5d7c113c29cc9bf91f69d42e..1824932328a43eca32e5fadfd89556d4799b3915 100644 --- a/crates/gpui/src/text_system/font_features.rs +++ b/crates/gpui/src/text_system/font_features.rs @@ -4,7 +4,9 @@ use schemars::{ }; macro_rules! create_definitions { - ($($(#[$meta:meta])* ($name:ident, $idx:expr)),* $(,)?) => { + ($($(#[$meta:meta])* ($name:ident, $idx:expr), ($comment:tt)),* $(,)?) => { + + /// The font features that can be configured for a given font. #[derive(Default, Copy, Clone, Eq, PartialEq, Hash)] pub struct FontFeatures { enabled: u64, @@ -13,6 +15,7 @@ macro_rules! create_definitions { impl FontFeatures { $( + /// $comment pub fn $name(&self) -> Option { if (self.enabled & (1 << $idx)) != 0 { Some(true) @@ -125,7 +128,7 @@ macro_rules! create_definitions { } create_definitions!( - (calt, 0), + (calt, 0, CALT), (case, 1), (cpsp, 2), (frac, 3), diff --git a/crates/gpui/src/window/element_cx.rs b/crates/gpui/src/window/element_cx.rs index 132cc72a5c7d344cf3dedbbc693ff2306b5bb8b9..c48374529ea90f7186c35f6f61e450ed16958bf0 100644 --- a/crates/gpui/src/window/element_cx.rs +++ b/crates/gpui/src/window/element_cx.rs @@ -1,3 +1,17 @@ +//! The element context is the main interface for interacting with the frame during a paint. +//! +//! Elements are hierarchical and with a few exceptions the context accumulates state in a stack +//! as it processes all of the elements in the frame. The methods that interact with this stack +//! are generally marked with `with_*`, and take a callback to denote the region of code that +//! should be executed with that state. +//! +//! The other main interface is the `paint_*` family of methods, which push basic drawing commands +//! to the GPU. Everything in a GPUI app is drawn with these methods. +//! +//! There are also several internal methds that GPUI uses, such as [`ElementContext::with_element_state`] +//! to call the paint and layout methods on elements. These have been included as they're often useful +//! for taking manual control of the layouting or painting of specialized elements. + use std::{ any::{Any, TypeId}, borrow::{Borrow, BorrowMut, Cow}, @@ -153,6 +167,9 @@ pub struct ElementContext<'a> { } impl<'a> WindowContext<'a> { + /// Convert this window context into an ElementContext in this callback. + /// If you need to use this method, you're probably intermixing the imperative + /// and declarative APIs, which is not recommended. pub fn with_element_context(&mut self, f: impl FnOnce(&mut ElementContext) -> R) -> R { f(&mut ElementContext { cx: WindowContext::new(self.app, self.window), @@ -338,6 +355,8 @@ impl<'a> ElementContext<'a> { self.window.next_frame.next_stacking_order_id = next_stacking_order_id; } + /// Push a text style onto the stack, and call a function with that style active. + /// Use [`AppContext::text_style`] to get the current, combined text style. pub fn with_text_style(&mut self, style: Option, f: F) -> R where F: FnOnce(&mut Self) -> R, @@ -979,9 +998,8 @@ impl<'a> ElementContext<'a> { self.window.layout_engine = Some(layout_engine); } - /// Obtain the bounds computed for the given LayoutId relative to the window. This method should not - /// be invoked until the paint phase begins, and will usually be invoked by GPUI itself automatically - /// in order to pass your element its `Bounds` automatically. + /// Obtain the bounds computed for the given LayoutId relative to the window. This method will usually be invoked by + /// GPUI itself automatically in order to pass your element its `Bounds` automatically. pub fn layout_bounds(&mut self, layout_id: LayoutId) -> Bounds { let mut bounds = self .window diff --git a/crates/refineable/derive_refineable/src/derive_refineable.rs b/crates/refineable/derive_refineable/src/derive_refineable.rs index bc906daece556106978f9788ff1502f004812180..294738b20eb93f3a8f6ab8886d57dbbca816e226 100644 --- a/crates/refineable/derive_refineable/src/derive_refineable.rs +++ b/crates/refineable/derive_refineable/src/derive_refineable.rs @@ -245,10 +245,14 @@ pub fn derive_refineable(input: TokenStream) -> TokenStream { } let gen = quote! { + /// A refinable version of [`#ident`], see that documentation for details. #[derive(Clone)] #derive_stream pub struct #refinement_ident #impl_generics { - #( #field_visibilities #field_names: #wrapped_types ),* + #( + #[allow(missing_docs)] + #field_visibilities #field_names: #wrapped_types + ),* } impl #impl_generics Refineable for #ident #ty_generics @@ -304,6 +308,7 @@ pub fn derive_refineable(input: TokenStream) -> TokenStream { impl #impl_generics #refinement_ident #ty_generics #where_clause { + /// Returns `true` if all fields are `Some` pub fn is_some(&self) -> bool { #( if self.#field_names.is_some() { From eab2e2112636c52e3b8217a9e2a7bd07ab0623db Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 22 Jan 2024 19:00:16 -0800 Subject: [PATCH 20/88] Document more styling functions --- crates/gpui/src/style.rs | 76 ++++++++++++++++++-- crates/gpui/src/styled.rs | 38 +++++++++- crates/gpui/src/text_system/font_features.rs | 8 +-- 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/crates/gpui/src/style.rs b/crates/gpui/src/style.rs index e1d82adea8751a8f0baf60c40dac4efbec993b1b..a12bb6df12836390855c6dfd6da078481425fabc 100644 --- a/crates/gpui/src/style.rs +++ b/crates/gpui/src/style.rs @@ -7,7 +7,7 @@ use crate::{ SizeRefinement, Styled, TextRun, }; use collections::HashSet; -use refineable::{Cascade, Refineable}; +use refineable::Refineable; use smallvec::SmallVec; pub use taffy::style::{ AlignContent, AlignItems, AlignSelf, Display, FlexDirection, FlexWrap, JustifyContent, @@ -15,10 +15,12 @@ pub use taffy::style::{ }; #[cfg(debug_assertions)] +/// Use this struct for interfacing with the 'debug_below' styling from your own elements. +/// If a parent element has this style set on it, then this struct will be set as a global in +/// GPUI. pub struct DebugBelow; -pub type StyleCascade = Cascade