git: Add global git integration enable/disable setting#43326
Conversation
08d1db3 to
8646946
Compare
351885c to
55845d6
Compare
Anthony-Eid
left a comment
There was a problem hiding this comment.
Great job so far! The code is structured well, I just want a few naming conventions to be inline with Zed's patterns before merging.
I'll be happy to pair on this next week, if you want to get this merged and #43173 merged as well. https://cal.com/anthony-eid/community-pairing
assets/settings/default.json
Outdated
| "enabled": { | ||
| // If true, all git integration features below are enabled or disabled independently. | ||
| // If false, all git integration features are disabled. | ||
| "global": true, | ||
| "status": true, | ||
| "diff": true | ||
| }, |
There was a problem hiding this comment.
Our settings have a pattern already for disabling an area of integration (e.g. disable_ai) where the format is disable_{area of integration name}. Could you please update the naming to reflect this?
Also, can you flatten out the fields so a user could just write
"git": {
"disable_git": true,
"enable_status": false // enable is used as the prefix here because it's not a whole area of integration
}There was a problem hiding this comment.
@Anthony-Eid , please check the link for the changes, thank you.
| #[with_fallible_options] | ||
| #[derive(Clone, Copy, Debug, PartialEq, Default, Serialize, Deserialize, JsonSchema, MergeFrom)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub struct GitEnabledSettings { | ||
| pub global: Option<bool>, | ||
| pub status: Option<bool>, | ||
| pub diff: Option<bool>, | ||
| } |
There was a problem hiding this comment.
You should be able to add a serde flatten attribute to flatten out this struct.
There was a problem hiding this comment.
@Anthony-Eid , please check the link for the changes, thank you.
|
Also, these settings need to be added to the settings UI in the git page. You can do that by adding it to the settings_data function in |
@Anthony-Eid , please check the link for the changes, use dynamic items to make the UI settings clearer. |
Hi @Anthony-Eid , I have scheduled time with you next week. Let's discuss then. Thank you. |
Anthony-Eid
left a comment
There was a problem hiding this comment.
Requested changes were made
|
Great work @leoliu0605 !! |
Closes #13304 Release Notes: - Add global `git status` and `git diff` on/off in one place instead of control everywhere We can first review to ensure this change meets both `Zed` and user requirements, as well as code rules. Currently, we only support user-level settings. We can wait for this PR: #43173 to be merged, then modify it to support both user and project levels.
…s#43326) Closes zed-industries#13304 Release Notes: - Add global `git status` and `git diff` on/off in one place instead of control everywhere We can first review to ensure this change meets both `Zed` and user requirements, as well as code rules. Currently, we only support user-level settings. We can wait for this PR: zed-industries#43173 to be merged, then modify it to support both user and project levels.
…s#43326) Closes zed-industries#13304 Release Notes: - Add global `git status` and `git diff` on/off in one place instead of control everywhere We can first review to ensure this change meets both `Zed` and user requirements, as well as code rules. Currently, we only support user-level settings. We can wait for this PR: zed-industries#43173 to be merged, then modify it to support both user and project levels.
…s#43326) Closes zed-industries#13304 Release Notes: - Add global `git status` and `git diff` on/off in one place instead of control everywhere We can first review to ensure this change meets both `Zed` and user requirements, as well as code rules. Currently, we only support user-level settings. We can wait for this PR: zed-industries#43173 to be merged, then modify it to support both user and project levels.
Closes #13304
Release Notes:
git statusandgit diffon/off in one place instead of control everywhereWe can first review to ensure this change meets both
Zedand user requirements, as well as code rules. Currently, we only support user-level settings. We can wait for this PR: #43173 to be merged, then modify it to support both user and project levels.