Skip to content

Commit 52ef008

Browse files
committed
🐛 Fix default provider configuration saving
Fix issue where default provider changes were not being saved to configuration files. - Modify apply_to_config to return boolean indicating changes made - Update apply_config_changes to track common parameter changes - Ensure provider insertion is handled only when needed - Add proper change tracking for all configuration updates - Fix duplicate provider setting logic in commands.rs
1 parent dea86d5 commit 52ef008

File tree

3 files changed

+70
-18
lines changed

3 files changed

+70
-18
lines changed

src/commands.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,16 @@ fn apply_config_changes(
4242
) -> anyhow::Result<bool> {
4343
let mut changes_made = false;
4444

45-
// Apply common parameters to the config
46-
common.apply_to_config(config)?;
45+
// Apply common parameters to the config and track if changes were made
46+
let common_changes = common.apply_to_config(config)?;
47+
changes_made |= common_changes;
4748

48-
// Handle provider change
49+
// Handle provider change - but skip if already handled by apply_to_config
4950
if let Some(provider) = &common.provider {
5051
if !get_available_provider_names().iter().any(|p| p == provider) {
5152
return Err(anyhow!("Invalid provider: {}", provider));
5253
}
53-
if config.default_provider != *provider {
54-
config.default_provider.clone_from(provider);
55-
changes_made = true;
56-
}
54+
// Only check for provider insertion if it wasn't already handled
5755
if !config.providers.contains_key(provider) {
5856
config
5957
.providers

src/commit/cli.rs

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ fn validate_pr_parameters(_from: Option<&String>, _to: Option<&String>) {
181181
// - from only: from..HEAD
182182
// - to only: main..to
183183
// - none: main..HEAD (caught earlier, but handled gracefully)
184-
184+
185185
// No validation errors needed - all combinations are handled
186186
}
187187

@@ -224,16 +224,44 @@ async fn generate_pr_based_on_parameters(
224224

225225
let pr_description = match (from, to) {
226226
(Some(from_ref), Some(to_ref)) => {
227-
handle_from_and_to_parameters(service, preset_str, &effective_instructions, from_ref, to_ref, &random_message).await?
227+
handle_from_and_to_parameters(
228+
service,
229+
preset_str,
230+
&effective_instructions,
231+
from_ref,
232+
to_ref,
233+
&random_message,
234+
)
235+
.await?
228236
}
229237
(None, Some(to_ref)) => {
230-
handle_to_only_parameter(service, preset_str, &effective_instructions, to_ref, &random_message).await?
238+
handle_to_only_parameter(
239+
service,
240+
preset_str,
241+
&effective_instructions,
242+
to_ref,
243+
&random_message,
244+
)
245+
.await?
231246
}
232247
(Some(from_ref), None) => {
233-
handle_from_only_parameter(service, preset_str, &effective_instructions, from_ref, &random_message).await?
248+
handle_from_only_parameter(
249+
service,
250+
preset_str,
251+
&effective_instructions,
252+
from_ref,
253+
&random_message,
254+
)
255+
.await?
234256
}
235257
(None, None) => {
236-
handle_no_parameters(service, preset_str, &effective_instructions, &random_message).await?
258+
handle_no_parameters(
259+
service,
260+
preset_str,
261+
&effective_instructions,
262+
&random_message,
263+
)
264+
.await?
237265
}
238266
};
239267

@@ -305,7 +333,7 @@ async fn handle_to_only_parameter(
305333
random_message: &crate::messages::ColoredMessage,
306334
) -> Result<super::types::GeneratedPullRequest> {
307335
let spinner = ui::create_spinner("");
308-
336+
309337
// Check if this is a single commit hash
310338
if is_likely_commit_hash(&to_ref) {
311339
// For a single commit specified with --to, compare it against its parent
@@ -359,7 +387,7 @@ async fn handle_from_only_parameter(
359387
random_message: &crate::messages::ColoredMessage,
360388
) -> Result<super::types::GeneratedPullRequest> {
361389
let spinner = ui::create_spinner("");
362-
390+
363391
// Check if this looks like a single commit hash that we should compare against its parent
364392
if is_likely_commit_hash(&from_ref) {
365393
// For a single commit hash, compare it against its parent (commit^..commit)

src/common.rs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,26 +75,52 @@ pub struct CommonParams {
7575
}
7676

7777
impl CommonParams {
78-
pub fn apply_to_config(&self, config: &mut Config) -> Result<()> {
78+
pub fn apply_to_config(&self, config: &mut Config) -> Result<bool> {
79+
let mut changes_made = false;
80+
7981
if let Some(provider) = &self.provider {
8082
// Convert "claude" to "anthropic" for backward compatibility
8183
let provider_name = if provider.to_lowercase() == "claude" {
8284
"anthropic".to_string()
8385
} else {
8486
provider.clone()
8587
};
86-
config.default_provider.clone_from(&provider_name);
88+
89+
// Check if we need to update the default provider
90+
if config.default_provider != provider_name {
91+
// Ensure the provider exists in the providers HashMap
92+
if !config.providers.contains_key(&provider_name) {
93+
// Import ProviderConfig here
94+
use crate::config::ProviderConfig;
95+
config.providers.insert(
96+
provider_name.clone(),
97+
ProviderConfig::default_for(&provider_name),
98+
);
99+
}
100+
101+
config.default_provider.clone_from(&provider_name);
102+
changes_made = true;
103+
}
87104
}
105+
88106
if let Some(instructions) = &self.instructions {
89107
config.set_temp_instructions(Some(instructions.clone()));
108+
// Note: temp instructions don't count as permanent changes
90109
}
110+
91111
if let Some(preset) = &self.preset {
92112
config.set_temp_preset(Some(preset.clone()));
113+
// Note: temp preset doesn't count as permanent changes
93114
}
115+
94116
if let Some(use_gitmoji) = self.gitmoji {
95-
config.use_gitmoji = use_gitmoji;
117+
if config.use_gitmoji != use_gitmoji {
118+
config.use_gitmoji = use_gitmoji;
119+
changes_made = true;
120+
}
96121
}
97-
Ok(())
122+
123+
Ok(changes_made)
98124
}
99125

100126
/// Check if the provided preset is valid for the specified preset type

0 commit comments

Comments
 (0)