From 3d68dba696232658ae036f54a9507fc9e4484381 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Wed, 12 Feb 2025 23:07:20 -0300 Subject: [PATCH 1/6] edit predictions: Iterate on onboarding modal copywriting (#24779) Release Notes: - N/A --------- Co-authored-by: Nathan Sobo <1789+nathansobo@users.noreply.github.com> --- assets/icons/zed_predict_bg.svg | 2 +- crates/zeta/src/onboarding_modal.rs | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/assets/icons/zed_predict_bg.svg b/assets/icons/zed_predict_bg.svg index 1332b2fdeceb0e..1dccbb51af0e61 100644 --- a/assets/icons/zed_predict_bg.svg +++ b/assets/icons/zed_predict_bg.svg @@ -1,4 +1,4 @@ - + diff --git a/crates/zeta/src/onboarding_modal.rs b/crates/zeta/src/onboarding_modal.rs index 9d7ad41a05aa0e..63aaa2b67200ce 100644 --- a/crates/zeta/src/onboarding_modal.rs +++ b/crates/zeta/src/onboarding_modal.rs @@ -168,7 +168,7 @@ impl Render for ZedPredictModal { .id("edit-prediction-onboarding") .key_context("ZedPredictModal") .relative() - .w(px(480.)) + .w(px(550.)) .h_full() .max_h(max_height) .p_4() @@ -201,7 +201,7 @@ impl Render for ZedPredictModal { svg() .path("icons/zed_predict_bg.svg") .text_color(cx.theme().colors().icon_disabled) - .w(px(460.)) + .w(px(530.)) .h(px(128.)) .overflow_hidden(), ), @@ -285,7 +285,9 @@ impl Render for ZedPredictModal { if self.user_store.read(cx).current_user().is_some() { let copy = match self.sign_in_status { - SignInStatus::Idle => "Get accurate and instant edit predictions at every keystroke. Before setting Zed as your edit prediction provider:", + SignInStatus::Idle => { + "Zed can now predict your next edit on every keystroke. Powered by Zeta, our open-source, open-dataset language model." + } SignInStatus::SignedIn => "Almost there! Ensure you:", SignInStatus::Waiting => unreachable!(), }; @@ -327,7 +329,7 @@ impl Render for ZedPredictModal { .child( Checkbox::new("tos-checkbox", self.terms_of_service.into()) .fill() - .label("Read and accept the") + .label("I have read and accept the") .on_click(cx.listener(move |this, state, _window, cx| { this.terms_of_service = *state == ToggleState::Selected; cx.notify(); @@ -351,7 +353,7 @@ impl Render for ZedPredictModal { "training-data-checkbox", self.data_collection_opted_in.into(), ) - .label("Open source repos: optionally share training data.") + .label("Contribute to the open dataset when editing open source.") .fill() .on_click(cx.listener( move |this, state, _window, cx| { @@ -393,14 +395,14 @@ impl Render for ZedPredictModal { ) ) .child(info_item( - "We ask this exclusively for open source projects.", + "We collect data exclusively from open source projects.", )) .child(info_item( "Zed automatically detects if your project is open source.", )) - .child(info_item("Toggle it anytime via the status bar menu.")) + .child(info_item("Toggle participation at any time via the status bar menu.")) .child(multiline_info_item( - "If turned on, this setting is valid for all open source projects", + "If turned on, this setting applies for all open source repositories", label_item("you open in Zed.") )) .child(multiline_info_item( @@ -425,7 +427,7 @@ impl Render for ZedPredictModal { .gap_2() .w_full() .child( - Button::new("accept-tos", "Enable Edit Predictions") + Button::new("accept-tos", "Enable Edit Prediction") .disabled(!self.terms_of_service) .style(ButtonStyle::Tinted(TintColor::Accent)) .full_width() From 71867096c88a70dc1609005baa4389bba9e5f548 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 12 Feb 2025 18:35:25 -0800 Subject: [PATCH 2/6] Migrate edit_prediction_provider setting before updating its value to 'zed' during onboarding (#24781) This fixes a bug where we'd update your settings to an invalid state if you were using the old `inline_completion_provider` setting, then onboarded to Zeta, then migrated your settings. Release Notes: - N/A Co-authored-by: Michael Sloan Co-authored-by: Agus Zubiaga --- Cargo.lock | 2 ++ crates/migrator/src/migrator.rs | 48 +++++++++++++++++------------ crates/zeta/Cargo.toml | 2 ++ crates/zeta/src/onboarding_modal.rs | 16 ++++++++++ 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9653529d7ecf80..a5661b189d9f4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17073,6 +17073,8 @@ dependencies = [ "language_models", "log", "menu", + "migrator", + "paths", "postage", "project", "regex", diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index c4eca930093df3..72576241f93582 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -68,6 +68,17 @@ pub fn migrate_settings(text: &str) -> Result> { ) } +pub fn migrate_edit_prediction_provider_settings(text: &str) -> Result> { + migrate( + &text, + &[( + SETTINGS_REPLACE_NESTED_KEY, + replace_edit_prediction_provider_setting, + )], + &EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY, + ) +} + type MigrationPatterns = &'static [( &'static str, fn(&str, &QueryMatch, &Query) -> Option<(Range, String)>, @@ -550,7 +561,10 @@ pub static CONTEXT_REPLACE: LazyLock> = LazyLock::new(|| { const SETTINGS_MIGRATION_PATTERNS: MigrationPatterns = &[ (SETTINGS_STRING_REPLACE_QUERY, replace_setting_name), - (SETTINGS_REPLACE_NESTED_KEY, replace_setting_nested_key), + ( + SETTINGS_REPLACE_NESTED_KEY, + replace_edit_prediction_provider_setting, + ), ( SETTINGS_REPLACE_IN_LANGUAGES_QUERY, replace_setting_in_languages, @@ -568,6 +582,14 @@ static SETTINGS_MIGRATION_QUERY: LazyLock = LazyLock::new(|| { .unwrap() }); +static EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY: LazyLock = LazyLock::new(|| { + Query::new( + &tree_sitter_json::LANGUAGE.into(), + SETTINGS_REPLACE_NESTED_KEY, + ) + .unwrap() +}); + const SETTINGS_STRING_REPLACE_QUERY: &str = r#"(document (object (pair @@ -622,7 +644,7 @@ const SETTINGS_REPLACE_NESTED_KEY: &str = r#" ) "#; -fn replace_setting_nested_key( +fn replace_edit_prediction_provider_setting( contents: &str, mat: &QueryMatch, query: &Query, @@ -641,27 +663,13 @@ fn replace_setting_nested_key( .byte_range(); let setting_name = contents.get(setting_range.clone())?; - let new_setting_name = SETTINGS_NESTED_STRING_REPLACE - .get(&parent_object_name)? - .get(setting_name)?; + if parent_object_name == "features" && setting_name == "inline_completion_provider" { + return Some((setting_range, "edit_prediction_provider".into())); + } - Some((setting_range, new_setting_name.to_string())) + None } -/// ```json -/// "features": { -/// "inline_completion_provider": "copilot" -/// }, -/// ``` -pub static SETTINGS_NESTED_STRING_REPLACE: LazyLock< - HashMap<&'static str, HashMap<&'static str, &'static str>>, -> = LazyLock::new(|| { - HashMap::from_iter([( - "features", - HashMap::from_iter([("inline_completion_provider", "edit_prediction_provider")]), - )]) -}); - const SETTINGS_REPLACE_IN_LANGUAGES_QUERY: &str = r#" (object (pair diff --git a/crates/zeta/Cargo.toml b/crates/zeta/Cargo.toml index 7e1f46c5fefa9e..f8f0bb4da3d494 100644 --- a/crates/zeta/Cargo.toml +++ b/crates/zeta/Cargo.toml @@ -36,6 +36,8 @@ language.workspace = true language_models.workspace = true log.workspace = true menu.workspace = true +migrator.workspace = true +paths.workspace = true postage.workspace = true project.workspace = true regex.workspace = true diff --git a/crates/zeta/src/onboarding_modal.rs b/crates/zeta/src/onboarding_modal.rs index 63aaa2b67200ce..a9ad7469e049ed 100644 --- a/crates/zeta/src/onboarding_modal.rs +++ b/crates/zeta/src/onboarding_modal.rs @@ -1,6 +1,7 @@ use std::{sync::Arc, time::Duration}; use crate::{onboarding_event, ZED_PREDICT_DATA_COLLECTION_CHOICE}; +use anyhow::Context as _; use client::{Client, UserStore}; use db::kvp::KEY_VALUE_STORE; use feature_flags::FeatureFlagAppExt as _; @@ -83,6 +84,7 @@ impl ZedPredictModal { let task = self .user_store .update(cx, |this, cx| this.accept_terms_of_service(cx)); + let fs = self.fs.clone(); cx.spawn(|this, mut cx| async move { task.await?; @@ -101,6 +103,20 @@ impl ZedPredictModal { .await .log_err(); + // Make sure edit prediction provider setting is using the new key + let settings_path = paths::settings_file().as_path(); + let settings_path = fs.canonicalize(settings_path).await.with_context(|| { + format!("Failed to canonicalize settings path {:?}", settings_path) + })?; + + if let Some(settings) = fs.load(&settings_path).await.log_err() { + if let Some(new_settings) = + migrator::migrate_edit_prediction_provider_settings(&settings)? + { + fs.atomic_write(settings_path, new_settings).await?; + } + } + this.update(&mut cx, |this, cx| { update_settings_file::(this.fs.clone(), cx, move |file, _| { file.features From 21a1541a70cae41a156cdf2a1ae964cce0054c20 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 12 Feb 2025 20:53:52 -0700 Subject: [PATCH 3/6] Branch/co-authors in commit (#24768) - **branch selector in commit box** - **TEMP** - **Add co-authors toggle button** Closes #ISSUE Release Notes: - N/A *or* Added/Fixed/Improved ... --------- Co-authored-by: Mikayla --- Cargo.lock | 2 +- crates/git_ui/Cargo.toml | 4 +- crates/git_ui/src/git_panel.rs | 257 ++++++++++++++++++--------------- 3 files changed, 144 insertions(+), 119 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a5661b189d9f4a..394c261556402c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5358,6 +5358,7 @@ dependencies = [ "fuzzy", "git", "gpui", + "itertools 0.14.0", "language", "menu", "multi_buffer", @@ -5372,7 +5373,6 @@ dependencies = [ "settings", "theme", "time", - "time_format", "ui", "util", "windows 0.58.0", diff --git a/crates/git_ui/Cargo.toml b/crates/git_ui/Cargo.toml index 19e443766a1429..215ee6b14a9c73 100644 --- a/crates/git_ui/Cargo.toml +++ b/crates/git_ui/Cargo.toml @@ -14,15 +14,16 @@ path = "src/git_ui.rs" [dependencies] anyhow.workspace = true +buffer_diff.workspace = true collections.workspace = true db.workspace = true -buffer_diff.workspace = true editor.workspace = true feature_flags.workspace = true futures.workspace = true fuzzy.workspace = true git.workspace = true gpui.workspace = true +itertools.workspace = true language.workspace = true menu.workspace = true multi_buffer.workspace = true @@ -37,7 +38,6 @@ serde_json.workspace = true settings.workspace = true theme.workspace = true time.workspace = true -time_format.workspace = true ui.workspace = true util.workspace = true workspace.workspace = true diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index cf7d77754e5628..7aa62062da5e4f 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -8,12 +8,13 @@ use collections::HashMap; use db::kvp::KEY_VALUE_STORE; use editor::commit_tooltip::CommitTooltip; use editor::{ - actions::MoveToEnd, scroll::ScrollbarAutoHide, Editor, EditorElement, EditorMode, - EditorSettings, MultiBuffer, ShowScrollbar, + scroll::ScrollbarAutoHide, Editor, EditorElement, EditorMode, EditorSettings, MultiBuffer, + ShowScrollbar, }; use git::repository::{CommitDetails, ResetMode}; use git::{repository::RepoPath, status::FileStatus, Commit, ToggleStaged}; use gpui::*; +use itertools::Itertools; use language::{markdown, Buffer, File, ParsedMarkdown}; use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev}; use multi_buffer::ExcerptInfo; @@ -27,8 +28,8 @@ use settings::Settings as _; use std::{collections::HashSet, path::PathBuf, sync::Arc, time::Duration, usize}; use time::OffsetDateTime; use ui::{ - prelude::*, ButtonLike, Checkbox, CheckboxWithLabel, Divider, DividerColor, ElevationIndex, - IndentGuideColors, ListItem, ListItemSpacing, Scrollbar, ScrollbarState, Tooltip, + prelude::*, ButtonLike, Checkbox, Divider, DividerColor, ElevationIndex, IndentGuideColors, + ListItem, ListItemSpacing, Scrollbar, ScrollbarState, Tooltip, }; use util::{maybe, ResultExt, TryFutureExt}; use workspace::{ @@ -45,7 +46,7 @@ actions!( OpenMenu, FocusEditor, FocusChanges, - FillCoAuthors, + ToggleFillCoAuthors, ] ); @@ -154,7 +155,7 @@ pub struct GitPanel { conflicted_count: usize, conflicted_staged_count: usize, current_modifiers: Modifiers, - enable_auto_coauthors: bool, + add_coauthors: bool, entries: Vec, entries_by_path: collections::HashMap, focus_handle: FocusHandle, @@ -260,7 +261,7 @@ impl GitPanel { conflicted_count: 0, conflicted_staged_count: 0, current_modifiers: window.modifiers(), - enable_auto_coauthors: true, + add_coauthors: true, entries: Vec::new(), entries_by_path: HashMap::default(), focus_handle: cx.focus_handle(), @@ -696,11 +697,14 @@ impl GitPanel { return; } - let message = self.commit_editor.read(cx).text(cx); + let mut message = self.commit_editor.read(cx).text(cx); if message.trim().is_empty() { self.commit_editor.read(cx).focus_handle(cx).focus(window); return; } + if self.add_coauthors { + self.fill_co_authors(&mut message, cx); + } let task = if self.has_staged_changes() { // Repository serializes all git operations, so we can just send a commit immediately @@ -781,38 +785,19 @@ impl GitPanel { self.pending_commit = Some(task); } - fn fill_co_authors(&mut self, _: &FillCoAuthors, window: &mut Window, cx: &mut Context) { - const CO_AUTHOR_PREFIX: &str = "Co-authored-by: "; + fn potential_co_authors(&self, cx: &App) -> Vec<(String, String)> { + let mut new_co_authors = Vec::new(); + let project = self.project.read(cx); let Some(room) = self .workspace .upgrade() .and_then(|workspace| workspace.read(cx).active_call()?.read(cx).room().cloned()) else { - return; + return Vec::default(); }; - let mut existing_text = self.commit_editor.read(cx).text(cx); - existing_text.make_ascii_lowercase(); - let lowercase_co_author_prefix = CO_AUTHOR_PREFIX.to_lowercase(); - let mut ends_with_co_authors = false; - let existing_co_authors = existing_text - .lines() - .filter_map(|line| { - let line = line.trim(); - if line.starts_with(&lowercase_co_author_prefix) { - ends_with_co_authors = true; - Some(line) - } else { - ends_with_co_authors = false; - None - } - }) - .collect::>(); - - let project = self.project.read(cx); let room = room.read(cx); - let mut new_co_authors = Vec::new(); for (peer_id, collaborator) in project.collaborators() { if collaborator.is_host { @@ -825,55 +810,87 @@ impl GitPanel { if participant.can_write() && participant.user.email.is_some() { let email = participant.user.email.clone().unwrap(); - if !existing_co_authors.contains(&email.as_ref()) { - new_co_authors.push(( - participant - .user - .name - .clone() - .unwrap_or_else(|| participant.user.github_login.clone()), - email, - )) - } + new_co_authors.push(( + participant + .user + .name + .clone() + .unwrap_or_else(|| participant.user.github_login.clone()), + email, + )) } } if !project.is_local() && !project.is_read_only(cx) { if let Some(user) = room.local_participant_user(cx) { if let Some(email) = user.email.clone() { - if !existing_co_authors.contains(&email.as_ref()) { - new_co_authors.push(( - user.name - .clone() - .unwrap_or_else(|| user.github_login.clone()), - email.clone(), - )) - } + new_co_authors.push(( + user.name + .clone() + .unwrap_or_else(|| user.github_login.clone()), + email.clone(), + )) } } } + new_co_authors + } + + fn toggle_fill_co_authors( + &mut self, + _: &ToggleFillCoAuthors, + _: &mut Window, + cx: &mut Context, + ) { + self.add_coauthors = !self.add_coauthors; + cx.notify(); + } + + fn fill_co_authors(&mut self, message: &mut String, cx: &mut Context) { + const CO_AUTHOR_PREFIX: &str = "Co-authored-by: "; + + let existing_text = message.to_ascii_lowercase(); + let lowercase_co_author_prefix = CO_AUTHOR_PREFIX.to_lowercase(); + let mut ends_with_co_authors = false; + let existing_co_authors = existing_text + .lines() + .filter_map(|line| { + let line = line.trim(); + if line.starts_with(&lowercase_co_author_prefix) { + ends_with_co_authors = true; + Some(line) + } else { + ends_with_co_authors = false; + None + } + }) + .collect::>(); + + let new_co_authors = self + .potential_co_authors(cx) + .into_iter() + .filter(|(_, email)| { + !existing_co_authors + .iter() + .any(|existing| existing.contains(email.as_str())) + }) + .collect::>(); + if new_co_authors.is_empty() { return; } - self.commit_editor.update(cx, |editor, cx| { - let editor_end = editor.buffer().read(cx).read(cx).len(); - let mut edit = String::new(); - if !ends_with_co_authors { - edit.push('\n'); - } - for (name, email) in new_co_authors { - edit.push('\n'); - edit.push_str(CO_AUTHOR_PREFIX); - edit.push_str(&name); - edit.push_str(" <"); - edit.push_str(&email); - edit.push('>'); - } - - editor.edit(Some((editor_end..editor_end, edit)), cx); - editor.move_to_end(&MoveToEnd, window, cx); - editor.focus_handle(cx).focus(window); - }); + if !ends_with_co_authors { + message.push('\n'); + } + for (name, email) in new_co_authors { + message.push('\n'); + message.push_str(CO_AUTHOR_PREFIX); + message.push_str(&name); + message.push_str(" <"); + message.push_str(&email); + message.push('>'); + } + message.push('\n'); } fn schedule_update( @@ -1046,11 +1063,6 @@ impl GitPanel { cx.notify(); } - fn toggle_auto_coauthors(&mut self, cx: &mut Context) { - self.enable_auto_coauthors = !self.enable_auto_coauthors; - cx.notify(); - } - fn header_state(&self, header_type: Section) -> ToggleState { let (staged_count, count) = match header_type { Section::New => (self.new_staged_count, self.new_count), @@ -1241,14 +1253,59 @@ impl GitPanel { cx.listener(move |this, _: &ClickEvent, window, cx| this.commit_changes(window, cx)) }); - let enable_coauthors = CheckboxWithLabel::new( - "enable-coauthors", - Label::new("Add Co-authors") - .color(Color::Disabled) - .size(LabelSize::XSmall), - self.enable_auto_coauthors.into(), - cx.listener(move |this, _, _, cx| this.toggle_auto_coauthors(cx)), - ); + let potential_co_authors = self.potential_co_authors(cx); + let enable_coauthors = if potential_co_authors.is_empty() { + None + } else { + Some( + IconButton::new("co-authors", IconName::Person) + .icon_color(Color::Disabled) + .selected_icon_color(Color::Selected) + .toggle_state(self.add_coauthors) + .tooltip(move |_, cx| { + let title = format!( + "Add co-authored-by:{}{}", + if potential_co_authors.len() == 1 { + "" + } else { + "\n" + }, + potential_co_authors + .iter() + .map(|(name, email)| format!(" {} <{}>", name, email)) + .join("\n") + ); + Tooltip::simple(title, cx) + }) + .on_click(cx.listener(|this, _, _, cx| { + this.add_coauthors = !this.add_coauthors; + cx.notify(); + })), + ) + }; + + let branch = self + .active_repository + .as_ref() + .and_then(|repo| repo.read(cx).branch().map(|b| b.name.clone())) + .unwrap_or_else(|| "".into()); + + let branch_selector = Button::new("branch-selector", branch) + .color(Color::Muted) + .style(ButtonStyle::Subtle) + .icon(IconName::GitBranch) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .size(ButtonSize::Compact) + .icon_position(IconPosition::Start) + .tooltip(Tooltip::for_action_title( + "Switch Branch", + &zed_actions::git::Branch, + )) + .on_click(cx.listener(|_, _, window, cx| { + window.dispatch_action(zed_actions::git::Branch.boxed_clone(), cx); + })) + .style(ButtonStyle::Transparent); let footer_size = px(32.); let gap = px(16.0); @@ -1274,7 +1331,7 @@ impl GitPanel { .left_2() .h(footer_size) .flex_none() - .child(enable_coauthors), + .child(branch_selector), ) .child( h_flex() @@ -1283,6 +1340,7 @@ impl GitPanel { .right_2() .h(footer_size) .flex_none() + .children(enable_coauthors) .child(commit_button), ) } @@ -1301,32 +1359,6 @@ impl GitPanel { }) { return None; } - - let _branch_selector = Button::new("branch-selector", branch.name.clone()) - .color(Color::Muted) - .style(ButtonStyle::Subtle) - .icon(IconName::GitBranch) - .icon_size(IconSize::Small) - .icon_color(Color::Muted) - .size(ButtonSize::Compact) - .icon_position(IconPosition::Start) - .tooltip(Tooltip::for_action_title( - "Switch Branch", - &zed_actions::git::Branch, - )) - .on_click(cx.listener(|_, _, window, cx| { - window.dispatch_action(zed_actions::git::Branch.boxed_clone(), cx); - })) - .style(ButtonStyle::Transparent); - - let _timestamp = Label::new(time_format::format_local_timestamp( - OffsetDateTime::from_unix_timestamp(commit.commit_timestamp).log_err()?, - OffsetDateTime::now_utc(), - time_format::TimestampFormat::Relative, - )) - .size(LabelSize::Small) - .color(Color::Muted); - let tooltip = if self.has_staged_changes() { "git reset HEAD^ --soft" } else { @@ -1374,13 +1406,6 @@ impl GitPanel { .icon_position(IconPosition::Start) .tooltip(Tooltip::for_action_title(tooltip, &git::Uncommit)) .on_click(cx.listener(|this, _, window, cx| this.uncommit(window, cx))), - // .child( - // panel_filled_button("Push") - // .icon(IconName::ArrowUp) - // .icon_size(IconSize::Small) - // .icon_color(Color::Muted) - // .icon_position(IconPosition::Start), // .disabled(true), - // ), ), ) } @@ -1857,7 +1882,7 @@ impl Render for GitPanel { .on_action(cx.listener(Self::focus_editor)) .on_action(cx.listener(Self::toggle_staged_for_selected)) .when(has_write_access && has_co_authors, |git_panel| { - git_panel.on_action(cx.listener(Self::fill_co_authors)) + git_panel.on_action(cx.listener(Self::toggle_fill_co_authors)) }) // .on_action(cx.listener(|this, &OpenSelected, cx| this.open_selected(&OpenSelected, cx))) .on_hover(cx.listener(|this, hovered, window, cx| { From 5d634245a2a6547a7d1dc4a3fcea768c0dff4e20 Mon Sep 17 00:00:00 2001 From: tidely <43219534+tidely@users.noreply.github.com> Date: Thu, 13 Feb 2025 05:55:22 +0200 Subject: [PATCH 4/6] remote_server: Remove unnecessary Box, prevent time-of-check time-of-use bug (#24730) The MultiWrite struct is defined in the function scope and is allowed to have a concrete type, which means we can throw away the extra Box. PathBuf::exists is known to be prone to invalid usage. It doesn't take into account permissions errors and just returns false, additionally it introduces a time-of-check time-of-use bug. While extremely unlikely, why not fix it anyway. Release Notes: - remove unnecessary Box - prevent time-of-check time-of-use bug --- crates/remote_server/src/unix.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/remote_server/src/unix.rs b/crates/remote_server/src/unix.rs index 71a770908abe78..88ffd1bbf63a2e 100644 --- a/crates/remote_server/src/unix.rs +++ b/crates/remote_server/src/unix.rs @@ -59,7 +59,7 @@ fn init_logging_proxy() { fn init_logging_server(log_file_path: PathBuf) -> Result>> { struct MultiWrite { - file: Box, + file: std::fs::File, channel: Sender>, buffer: Vec, } @@ -80,14 +80,11 @@ fn init_logging_server(log_file_path: PathBuf) -> Result>> { } } - let log_file = Box::new(if log_file_path.exists() { - std::fs::OpenOptions::new() - .append(true) - .open(&log_file_path) - .context("Failed to open log file in append mode")? - } else { - std::fs::File::create(&log_file_path).context("Failed to create log file")? - }); + let log_file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&log_file_path) + .context("Failed to open log file in append mode")?; let (tx, rx) = smol::channel::unbounded(); From fc7bf7bcb92c062c469e572071625418f033b5cc Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 12 Feb 2025 23:14:45 -0500 Subject: [PATCH 5/6] Bump Zed to v0.175 (#24785) Release Notes: -N/A --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 394c261556402c..2c2036f9c0a8db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16626,7 +16626,7 @@ dependencies = [ [[package]] name = "zed" -version = "0.174.0" +version = "0.175.0" dependencies = [ "activity_indicator", "anyhow", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 69545a697bbeca..bb7c5d3c291188 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -2,7 +2,7 @@ description = "The fast, collaborative code editor." edition.workspace = true name = "zed" -version = "0.174.0" +version = "0.175.0" publish.workspace = true license = "GPL-3.0-or-later" authors = ["Zed Team "] From d57f5937d451dc1d5c771a8ade63ccb47e73add6 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 12 Feb 2025 22:26:34 -0700 Subject: [PATCH 6/6] Git panel: Right click menu (#24787) Release Notes: - N/A --- crates/editor/src/editor.rs | 41 ++-- crates/git_ui/src/git_panel.rs | 390 ++++++++++++++++++++------------- crates/project/src/project.rs | 10 + 3 files changed, 278 insertions(+), 163 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index f77a32a92bbcff..29220f292b2330 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -67,7 +67,10 @@ use element::{AcceptEditPredictionBinding, LineWithInvisibles, PositionMap}; pub use element::{ CursorLayout, EditorElement, HighlightedRange, HighlightedRangeLine, PointForPosition, }; -use futures::{future, FutureExt}; +use futures::{ + future::{self, Shared}, + FutureExt, +}; use fuzzy::StringMatchCandidate; use code_context_menus::{ @@ -761,6 +764,7 @@ pub struct Editor { next_scroll_position: NextScrollCursorCenterTopBottom, addons: HashMap>, registered_buffers: HashMap, + load_diff_task: Option>>, selection_mark_mode: bool, toggle_fold_multiple_buffers: Task<()>, _scroll_cursor_center_top_bottom_task: Task<()>, @@ -1318,12 +1322,16 @@ impl Editor { }; let mut code_action_providers = Vec::new(); + let mut load_uncommitted_diff = None; if let Some(project) = project.clone() { - get_uncommitted_diff_for_buffer( - &project, - buffer.read(cx).all_buffers(), - buffer.clone(), - cx, + load_uncommitted_diff = Some( + get_uncommitted_diff_for_buffer( + &project, + buffer.read(cx).all_buffers(), + buffer.clone(), + cx, + ) + .shared(), ); code_action_providers.push(Rc::new(project) as Rc<_>); } @@ -1471,6 +1479,7 @@ impl Editor { selection_mark_mode: false, toggle_fold_multiple_buffers: Task::ready(()), text_style_refinement: None, + load_diff_task: load_uncommitted_diff, }; this.tasks_update_task = Some(this.refresh_runnables(window, cx)); this._subscriptions.extend(project_subscriptions); @@ -14120,11 +14129,14 @@ impl Editor { let buffer_id = buffer.read(cx).remote_id(); if self.buffer.read(cx).diff_for(buffer_id).is_none() { if let Some(project) = &self.project { - get_uncommitted_diff_for_buffer( - project, - [buffer.clone()], - self.buffer.clone(), - cx, + self.load_diff_task = Some( + get_uncommitted_diff_for_buffer( + project, + [buffer.clone()], + self.buffer.clone(), + cx, + ) + .shared(), ); } } @@ -14879,6 +14891,10 @@ impl Editor { gpui::Size::new(em_width, line_height) } + + pub fn wait_for_diff_to_load(&self) -> Option>> { + self.load_diff_task.clone() + } } fn get_uncommitted_diff_for_buffer( @@ -14886,7 +14902,7 @@ fn get_uncommitted_diff_for_buffer( buffers: impl IntoIterator>, buffer: Entity, cx: &mut App, -) { +) -> Task<()> { let mut tasks = Vec::new(); project.update(cx, |project, cx| { for buffer in buffers { @@ -14903,7 +14919,6 @@ fn get_uncommitted_diff_for_buffer( }) .ok(); }) - .detach(); } fn char_len_with_expanded_tabs(offset: usize, text: &str, tab_size: NonZeroU32) -> usize { diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 7aa62062da5e4f..e7e57a3cb312b6 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -16,7 +16,7 @@ use git::{repository::RepoPath, status::FileStatus, Commit, ToggleStaged}; use gpui::*; use itertools::Itertools; use language::{markdown, Buffer, File, ParsedMarkdown}; -use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev}; +use menu::{Confirm, SecondaryConfirm, SelectFirst, SelectLast, SelectNext, SelectPrev}; use multi_buffer::ExcerptInfo; use panel::{panel_editor_container, panel_editor_style, panel_filled_button, PanelHeader}; use project::{ @@ -28,10 +28,11 @@ use settings::Settings as _; use std::{collections::HashSet, path::PathBuf, sync::Arc, time::Duration, usize}; use time::OffsetDateTime; use ui::{ - prelude::*, ButtonLike, Checkbox, Divider, DividerColor, ElevationIndex, IndentGuideColors, - ListItem, ListItemSpacing, Scrollbar, ScrollbarState, Tooltip, + prelude::*, ButtonLike, Checkbox, ContextMenu, Divider, DividerColor, ElevationIndex, ListItem, + ListItemSpacing, Scrollbar, ScrollbarState, Tooltip, }; use util::{maybe, ResultExt, TryFutureExt}; +use workspace::SaveIntent; use workspace::{ dock::{DockPosition, Panel, PanelEvent}, notifications::{DetachAndPromptErr, NotificationId}, @@ -79,7 +80,6 @@ pub fn init(cx: &mut App) { #[derive(Debug, Clone)] pub enum Event { Focus, - OpenedEntry { path: ProjectPath }, } #[derive(Serialize, Deserialize)] @@ -112,7 +112,7 @@ impl GitHeaderEntry { pub fn title(&self) -> &'static str { match self.header { Section::Conflict => "Conflicts", - Section::Tracked => "Changed", + Section::Tracked => "Changes", Section::New => "New", } } @@ -177,6 +177,7 @@ pub struct GitPanel { update_visible_entries_task: Task<()>, width: Option, workspace: WeakEntity, + context_menu: Option<(Entity, Point, Subscription)>, } fn commit_message_editor( @@ -215,7 +216,7 @@ impl GitPanel { let active_repository = project.read(cx).active_repository(cx); let workspace = cx.entity().downgrade(); - let git_panel = cx.new(|cx| { + cx.new(|cx| { let focus_handle = cx.focus_handle(); cx.on_focus(&focus_handle, window, Self::focus_in).detach(); cx.on_focus_out(&focus_handle, window, |this, _, window, cx| { @@ -282,30 +283,13 @@ impl GitPanel { tracked_staged_count: 0, update_visible_entries_task: Task::ready(()), width: Some(px(360.)), + context_menu: None, workspace, }; git_panel.schedule_update(false, window, cx); git_panel.show_scrollbar = git_panel.should_show_scrollbar(cx); git_panel - }); - - cx.subscribe_in( - &git_panel, - window, - move |workspace, _, event: &Event, window, cx| match event.clone() { - Event::OpenedEntry { path } => { - workspace - .open_path_preview(path, None, false, false, window, cx) - .detach_and_prompt_err("Failed to open file", window, cx, |e, _, _| { - Some(format!("{e}")) - }); - } - Event::Focus => { /* TODO */ } - }, - ) - .detach(); - - git_panel + }) } pub fn select_entry_by_path( @@ -468,7 +452,7 @@ impl GitPanel { fn select_first(&mut self, _: &SelectFirst, _window: &mut Window, cx: &mut Context) { if self.entries.first().is_some() { - self.selected_entry = Some(0); + self.selected_entry = Some(1); self.scroll_to_selected_entry(cx); } } @@ -486,7 +470,16 @@ impl GitPanel { selected_entry }; - self.selected_entry = Some(new_selected_entry); + if matches!( + self.entries.get(new_selected_entry), + Some(GitListEntry::Header(..)) + ) { + if new_selected_entry > 0 { + self.selected_entry = Some(new_selected_entry - 1) + } + } else { + self.selected_entry = Some(new_selected_entry); + } self.scroll_to_selected_entry(cx); } @@ -506,8 +499,14 @@ impl GitPanel { } else { selected_entry }; - - self.selected_entry = Some(new_selected_entry); + if matches!( + self.entries.get(new_selected_entry), + Some(GitListEntry::Header(..)) + ) { + self.selected_entry = Some(new_selected_entry + 1); + } else { + self.selected_entry = Some(new_selected_entry); + } self.scroll_to_selected_entry(cx); } @@ -537,7 +536,7 @@ impl GitPanel { active_repository.read(cx).entry_count() > 0 }); if have_entries && self.selected_entry.is_none() { - self.selected_entry = Some(0); + self.selected_entry = Some(1); self.scroll_to_selected_entry(cx); cx.notify(); } @@ -559,7 +558,7 @@ impl GitPanel { self.selected_entry.and_then(|i| self.entries.get(i)) } - fn open_selected(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context) { + fn open_diff(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context) { maybe!({ let entry = self.entries.get(self.selected_entry?)?.status_entry()?; @@ -572,6 +571,121 @@ impl GitPanel { self.focus_handle.focus(window); } + fn open_file( + &mut self, + _: &menu::SecondaryConfirm, + window: &mut Window, + cx: &mut Context, + ) { + maybe!({ + let entry = self.entries.get(self.selected_entry?)?.status_entry()?; + let active_repo = self.active_repository.as_ref()?; + let path = active_repo + .read(cx) + .repo_path_to_project_path(&entry.repo_path)?; + if entry.status.is_deleted() { + return None; + } + + self.workspace + .update(cx, |workspace, cx| { + workspace + .open_path_preview(path, None, false, false, window, cx) + .detach_and_prompt_err("Failed to open file", window, cx, |e, _, _| { + Some(format!("{e}")) + }); + }) + .ok() + }); + } + + fn revert( + &mut self, + _: &editor::actions::RevertFile, + window: &mut Window, + cx: &mut Context, + ) { + maybe!({ + let list_entry = self.entries.get(self.selected_entry?)?.clone(); + let entry = list_entry.status_entry()?; + let active_repo = self.active_repository.as_ref()?; + let path = active_repo + .read(cx) + .repo_path_to_project_path(&entry.repo_path)?; + let workspace = self.workspace.clone(); + + if entry.status.is_staged() != Some(false) { + self.update_staging_area_for_entries(false, vec![entry.repo_path.clone()], cx); + } + + if entry.status.is_created() { + let prompt = window.prompt( + PromptLevel::Info, + "Do you want to trash this file?", + None, + &["Trash", "Cancel"], + cx, + ); + cx.spawn_in(window, |_, mut cx| async move { + match prompt.await { + Ok(0) => {} + _ => return Ok(()), + } + let task = workspace.update(&mut cx, |workspace, cx| { + workspace + .project() + .update(cx, |project, cx| project.delete_file(path, true, cx)) + })?; + if let Some(task) = task { + task.await?; + } + Ok(()) + }) + .detach_and_prompt_err( + "Failed to trash file", + window, + cx, + |e, _, _| Some(format!("{e}")), + ); + return Some(()); + } + + let open_path = workspace.update(cx, |workspace, cx| { + workspace.open_path_preview(path, None, true, false, window, cx) + }); + + cx.spawn_in(window, |_, mut cx| async move { + let item = open_path?.await?; + let editor = cx.update(|_, cx| { + item.act_as::(cx) + .ok_or_else(|| anyhow::anyhow!("didn't open editor")) + })??; + + if let Some(task) = + editor.update(&mut cx, |editor, _| editor.wait_for_diff_to_load())? + { + task.await + }; + + editor.update_in(&mut cx, |editor, window, cx| { + editor.revert_file(&Default::default(), window, cx); + })?; + + workspace + .update_in(&mut cx, |workspace, window, cx| { + workspace.save_active_item(SaveIntent::Save, window, cx) + })? + .await?; + Ok(()) + }) + .detach_and_prompt_err("Failed to open file", window, cx, |e, _, _| { + Some(format!("{e}")) + }); + + Some(()) + }); + } + fn toggle_staged_for_entry( &mut self, entry: &GitListEntry, @@ -606,7 +720,18 @@ impl GitPanel { (goal_staged_state, entries) } }; + self.update_staging_area_for_entries(stage, repo_paths, cx); + } + fn update_staging_area_for_entries( + &mut self, + stage: bool, + repo_paths: Vec, + cx: &mut Context, + ) { + let Some(active_repository) = self.active_repository.clone() else { + return; + }; let op_id = self.pending.iter().map(|p| p.op_id).max().unwrap_or(0) + 1; self.pending.push(PendingOperation { op_id, @@ -615,7 +740,6 @@ impl GitPanel { finished: false, }); let repo_paths = repo_paths.clone(); - let active_repository = active_repository.clone(); let repository = active_repository.read(cx); self.update_counts(repository); cx.notify(); @@ -1530,7 +1654,7 @@ impl GitPanel { fn render_entries( &self, has_write_access: bool, - window: &Window, + _: &Window, cx: &mut Context, ) -> impl IntoElement { let entry_count = self.entries.len(); @@ -1571,61 +1695,6 @@ impl GitPanel { items } }) - .with_decoration( - ui::indent_guides( - cx.entity().clone(), - self.indent_size(window, cx), - IndentGuideColors::panel(cx), - |this, range, _windows, _cx| { - this.entries - .iter() - .skip(range.start) - .map(|entry| match entry { - GitListEntry::GitStatusEntry(_) => 1, - GitListEntry::Header(_) => 0, - }) - .collect() - }, - ) - .with_render_fn( - cx.entity().clone(), - move |_, params, _, _| { - let indent_size = params.indent_size; - let left_offset = indent_size - px(3.0); - let item_height = params.item_height; - - params - .indent_guides - .into_iter() - .enumerate() - .map(|(_, layout)| { - let offset = if layout.continues_offscreen { - px(0.) - } else { - px(4.0) - }; - let bounds = Bounds::new( - point( - px(layout.offset.x as f32) * indent_size + left_offset, - px(layout.offset.y as f32) * item_height + offset, - ), - size( - px(1.), - px(layout.length as f32) * item_height - - px(offset.0 * 2.), - ), - ); - ui::RenderedIndentGuide { - bounds, - layout, - is_active: false, - hitbox: None, - } - }) - .collect() - }, - ), - ) .size_full() .with_sizing_behavior(ListSizingBehavior::Infer) .with_horizontal_sizing_behavior(ListHorizontalSizingBehavior::Unconstrained) @@ -1642,59 +1711,22 @@ impl GitPanel { &self, ix: usize, header: &GitHeaderEntry, - has_write_access: bool, - window: &Window, - cx: &Context, + _: bool, + _: &Window, + _: &Context, ) -> AnyElement { - let selected = self.selected_entry == Some(ix); - let header_state = if self.has_staged_changes() { - self.header_state(header.header) - } else { - match header.header { - Section::Tracked | Section::Conflict => ToggleState::Selected, - Section::New => ToggleState::Unselected, - } - }; - - let checkbox = Checkbox::new(("checkbox", ix), header_state) - .disabled(!has_write_access) - .fill() - .placeholder(!self.has_staged_changes()) - .elevation(ElevationIndex::Surface) - .on_click({ - let header = header.clone(); - cx.listener(move |this, _, window, cx| { - this.toggle_staged_for_entry(&GitListEntry::Header(header.clone()), window, cx); - cx.stop_propagation(); - }) - }); - - let start_slot = h_flex() - .id(("start-slot", ix)) - .gap(DynamicSpacing::Base04.rems(cx)) - .child(checkbox) - .tooltip(|window, cx| Tooltip::for_action("Stage File", &ToggleStaged, window, cx)) - .on_mouse_down(MouseButton::Left, |_, _, cx| { - // prevent the list item active state triggering when toggling checkbox - cx.stop_propagation(); - }); - div() .w_full() .child( ListItem::new(ix) .spacing(ListItemSpacing::Sparse) - .start_slot(start_slot) - .toggle_state(selected) - .focused(selected && self.focus_handle(cx).is_focused(window)) - .disabled(!has_write_access) - .on_click({ - cx.listener(move |this, _, _, cx| { - this.selected_entry = Some(ix); - cx.notify(); - }) - }) - .child(h_flex().child(self.entry_label(header.title(), Color::Muted))), + .disabled(true) + .child( + Label::new(header.title()) + .color(Color::Muted) + .size(LabelSize::Small) + .single_line(), + ), ) .into_any_element() } @@ -1710,6 +1742,50 @@ impl GitPanel { repo.update(cx, |repo, cx| repo.show(sha, cx)) } + fn deploy_context_menu( + &mut self, + position: Point, + ix: usize, + window: &mut Window, + cx: &mut Context, + ) { + let Some(entry) = self.entries.get(ix).and_then(|e| e.status_entry()) else { + return; + }; + let revert_title = if entry.status.is_deleted() { + "Restore file" + } else if entry.status.is_created() { + "Trash file" + } else { + "Discard changes" + }; + let context_menu = ContextMenu::build(window, cx, |context_menu, _, _| { + context_menu + .action("Stage File", ToggleStaged.boxed_clone()) + .action(revert_title, editor::actions::RevertFile.boxed_clone()) + .separator() + .action("Open Diff", Confirm.boxed_clone()) + .action("Open File", SecondaryConfirm.boxed_clone()) + }); + + let subscription = cx.subscribe_in( + &context_menu, + window, + |this, _, _: &DismissEvent, window, cx| { + if this.context_menu.as_ref().is_some_and(|context_menu| { + context_menu.0.focus_handle(cx).contains_focused(window, cx) + }) { + cx.focus_self(window); + } + this.context_menu.take(); + cx.notify(); + }, + ); + self.selected_entry = Some(ix); + self.context_menu = Some((context_menu, position, subscription)); + cx.notify(); + } + fn render_entry( &self, ix: usize, @@ -1789,26 +1865,31 @@ impl GitPanel { cx.stop_propagation(); }); - let id = ElementId::Name(format!("entry_{}", display_name).into()); - div() .w_full() .child( - ListItem::new(id) - .indent_level(1) - .indent_step_size(Checkbox::container_size(cx).to_pixels(window.rem_size())) + ListItem::new(ix) .spacing(ListItemSpacing::Sparse) .start_slot(start_slot) .toggle_state(selected) .focused(selected && self.focus_handle(cx).is_focused(window)) .disabled(!has_write_access) .on_click({ - cx.listener(move |this, _, window, cx| { + cx.listener(move |this, event: &ClickEvent, window, cx| { this.selected_entry = Some(ix); cx.notify(); - this.open_selected(&Default::default(), window, cx); + if event.modifiers().secondary() { + this.open_file(&Default::default(), window, cx) + } else { + this.open_diff(&Default::default(), window, cx); + } }) }) + .on_secondary_mouse_down(cx.listener( + move |this, event: &MouseDownEvent, window, cx| { + this.deploy_context_menu(event.position, ix, window, cx) + }, + )) .child( h_flex() .when_some(repo_path.parent(), |this, parent| { @@ -1870,14 +1951,14 @@ impl Render for GitPanel { })) .on_action(cx.listener(GitPanel::commit)) }) - .when(self.is_focused(window, cx), |this| { - this.on_action(cx.listener(Self::select_first)) - .on_action(cx.listener(Self::select_next)) - .on_action(cx.listener(Self::select_prev)) - .on_action(cx.listener(Self::select_last)) - .on_action(cx.listener(Self::close_panel)) - }) - .on_action(cx.listener(Self::open_selected)) + .on_action(cx.listener(Self::select_first)) + .on_action(cx.listener(Self::select_next)) + .on_action(cx.listener(Self::select_prev)) + .on_action(cx.listener(Self::select_last)) + .on_action(cx.listener(Self::close_panel)) + .on_action(cx.listener(Self::open_diff)) + .on_action(cx.listener(Self::open_file)) + .on_action(cx.listener(Self::revert)) .on_action(cx.listener(Self::focus_changes_list)) .on_action(cx.listener(Self::focus_editor)) .on_action(cx.listener(Self::toggle_staged_for_selected)) @@ -1906,6 +1987,15 @@ impl Render for GitPanel { }) .children(self.render_previous_commit(cx)) .child(self.render_commit_editor(window, cx)) + .children(self.context_menu.as_ref().map(|(menu, position, _)| { + deferred( + anchored() + .position(*position) + .anchor(gpui::Corner::TopLeft) + .child(menu.clone()), + ) + .with_priority(1) + })) } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 8cc0481a5a6981..e369c040ccdfb6 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1612,6 +1612,16 @@ impl Project { }) } + pub fn delete_file( + &mut self, + path: ProjectPath, + trash: bool, + cx: &mut Context, + ) -> Option>> { + let entry = self.entry_for_path(&path, cx)?; + self.delete_entry(entry.id, trash, cx) + } + pub fn delete_entry( &mut self, entry_id: ProjectEntryId,