Finish the rest of the todos

Anthony , Ben Kunkle , and Conrad Irwin created 2 months ago

Co-authored-by: Ben Kunkle <[e-mail]>
Co-authored-by: Conrad Irwin <[e-mail]>

Change summary

crates/agent_settings/src/agent_settings.rs   | 1 -
crates/language_models/src/provider/vercel.rs | 1 -
crates/project/src/project_settings.rs        | 5 ++++-
crates/settings/src/settings_store.rs         | 1 -
crates/terminal/src/terminal_settings.rs      | 1 -
5 files changed, 4 insertions(+), 5 deletions(-)

Detailed changes

crates/agent_settings/src/agent_settings.rs

@@ -147,7 +147,6 @@ impl Default for AgentProfileId {
 }
 
 impl Settings for AgentSettings {
-    // todo!() test preserved keys logic
     fn from_defaults(content: &settings::SettingsContent, _cx: &mut App) -> Self {
         let agent = content.agent.clone().unwrap();
         Self {

crates/language_models/src/provider/vercel.rs

@@ -25,7 +25,6 @@ use crate::{api_key::ApiKeyState, ui::InstructionListItem};
 const PROVIDER_ID: LanguageModelProviderId = LanguageModelProviderId::new("vercel");
 const PROVIDER_NAME: LanguageModelProviderName = LanguageModelProviderName::new("Vercel");
 
-// todo!() -> Remove default implementation
 const API_KEY_ENV_VAR_NAME: &str = "VERCEL_API_KEY";
 static API_KEY_ENV_VAR: LazyLock<EnvVar> = env_var!(API_KEY_ENV_VAR_NAME);
 

crates/project/src/project_settings.rs

@@ -47,7 +47,10 @@ pub struct ProjectSettings {
     /// To override settings for a language, add an entry for that language server's
     /// name to the lsp value.
     /// Default: null
-    // todo! should these hash map types be Map<key, SettingsContent> or Map<Key, Settings>
+    // todo(settings-follow-up)
+    // We should change to use a non content type (settings::LspSettings is a content type)
+    // Note: Will either require merging with defaults, which also requires deciding where the defaults come from,
+    //       or case by case deciding which fields are optional and which are actually required.
     pub lsp: HashMap<LanguageServerName, settings::LspSettings>,
 
     /// Common language server settings.

crates/settings/src/settings_store.rs

@@ -1203,7 +1203,6 @@ mod tests {
             TitleBarVisibility::Never
         );
 
-        // todo!()
         store
             .set_local_settings(
                 WorktreeId::from_usize(1),

crates/terminal/src/terminal_settings.rs

@@ -250,7 +250,6 @@ impl settings::Settings for TerminalSettings {
                 }
             }
         }
-        // todo!() test that this works.
         if content.terminal.is_none() && default != TerminalSettingsContent::default() {
             content.terminal = Some(default)
         }