From 36ab2db99841fbe373ae37cb86484d6ff25d1455 Mon Sep 17 00:00:00 2001 From: codcod Date: Thu, 13 Nov 2025 21:18:48 +0100 Subject: [PATCH] refactor: consolidate config yaml creation --- plugins/repos-validate/src/main.rs | 27 +---- src/config/loader.rs | 187 ++++++++++++++++++++++++----- src/lib.rs | 1 + 3 files changed, 164 insertions(+), 51 deletions(-) diff --git a/plugins/repos-validate/src/main.rs b/plugins/repos-validate/src/main.rs index 933d970..680e3fa 100644 --- a/plugins/repos-validate/src/main.rs +++ b/plugins/repos-validate/src/main.rs @@ -1,7 +1,7 @@ use anyhow::{Context, Result}; use clap::Parser; use colored::Colorize; -use repos::{Repository, is_debug_mode, load_plugin_context}; +use repos::{Repository, is_debug_mode, load_plugin_context, save_config}; use repos_github::GitHubClient; use std::collections::{HashMap, HashSet}; use std::fs; @@ -268,28 +268,9 @@ fn apply_sync(config_path: &PathBuf, sync_map: &HashMap) -> R } } - // Write back to file - let yaml = serde_yaml::to_string(&config).context("Failed to serialize updated config")?; - - // Minimal fix for yamllint: indent array items under 'repositories:' and 'recipes:' - let fixed_yaml = yaml - .lines() - .map(|line| { - // Indent array items and their properties - if line.starts_with("- ") || (line.starts_with(" ") && !line.starts_with(" ")) { - format!(" {}", line) - } else { - line.to_string() - } - }) - .collect::>() - .join("\n"); - - // Add document marker for yamllint compliance - let updated_content = format!("---\n{}\n", fixed_yaml); - - fs::write(config_path, &updated_content) - .context(format!("Failed to write config file: {:?}", config_path))?; + // Write back to file using centralized save_config function + save_config(&config, config_path.to_str().unwrap()) + .context("Failed to write updated config")?; println!("{} Successfully updated config.yaml", "✅".green()); println!(" {} repositories were synchronized", sync_map.len()); diff --git a/src/config/loader.rs b/src/config/loader.rs index 51d8daf..ccd5855 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -44,34 +44,7 @@ impl Config { /// Save configuration to a file pub fn save(&self, path: &str) -> Result<()> { - // Use standard serde_yaml serialization - let yaml = serde_yaml::to_string(self)?; - - // Minimal fix for yamllint: indent array items under 'repositories:' and 'recipes:' - // This is the only formatting adjustment needed for yamllint compliance - let fixed_yaml = yaml - .lines() - .map(|line| { - // If line starts with "- " and previous non-empty line ends with ":" - // then it's an array item that needs indenting - if line.starts_with("- ") { - format!(" {}", line) - } else if line.starts_with(" ") && !line.starts_with(" ") { - // Lines with 1-2 spaces are properties of array items, need to be 4 spaces - format!(" {}", line) - } else { - line.to_string() - } - }) - .collect::>() - .join("\n"); - - // Add document marker for yamllint compliance - let yaml_content = format!("---\n{}\n", fixed_yaml); - - std::fs::write(path, yaml_content)?; - - Ok(()) + save_config(self, path) } /// Filter repositories by specific names @@ -185,6 +158,92 @@ impl Default for Config { } } +/// Save a config to a file with proper YAML formatting and comment preservation +/// +/// This is the centralized function for writing config.yaml files. It ensures: +/// - Leading comments are preserved +/// - YAML document start marker (---) is added after comments +/// - Proper indentation for yamllint compliance +/// - Trailing newline +/// +/// Use this function or Config::save() for all config file writes to ensure consistency. +pub fn save_config(config: &T, path: &str) -> Result<()> { + // Read existing file to preserve leading comments + let existing_comments = if Path::new(path).exists() { + extract_leading_comments(path)? + } else { + Vec::new() + }; + + // Serialize config to YAML + let yaml = serde_yaml::to_string(config)?; + + // Apply minimal indentation fix for yamllint compliance + let fixed_yaml = yaml + .lines() + .map(|line| { + if line.starts_with("- ") || (line.starts_with(" ") && !line.starts_with(" ")) { + format!(" {}", line) + } else { + line.to_string() + } + }) + .collect::>() + .join("\n"); + + // Combine comments, document marker, and content + let yaml_content = add_document_start_preserving_comments(&existing_comments, &fixed_yaml); + + std::fs::write(path, yaml_content)?; + + Ok(()) +} + +/// Extract leading comments from a YAML file +fn extract_leading_comments(path: &str) -> Result> { + let content = std::fs::read_to_string(path)?; + let mut comments = Vec::new(); + + for line in content.lines() { + let trimmed = line.trim(); + if trimmed.starts_with('#') { + comments.push(line.to_string()); + } else if trimmed == "---" { + // Stop at document start marker + break; + } else if !trimmed.is_empty() { + // Stop at first non-comment, non-empty line + break; + } + } + + Ok(comments) +} + +/// Add document start marker while preserving leading comments +fn add_document_start_preserving_comments(comments: &[String], yaml: &str) -> String { + let mut result = String::new(); + + // Add leading comments + for comment in comments { + result.push_str(comment); + result.push('\n'); + } + + // Add document start marker + result.push_str("---\n"); + + // Add YAML content + result.push_str(yaml); + + // Ensure trailing newline + if !result.ends_with('\n') { + result.push('\n'); + } + + result +} + #[cfg(test)] mod tests { use super::*; @@ -553,4 +612,76 @@ mod tests { assert_eq!(filtered.len(), 1); assert_eq!(filtered[0].name, "repo2"); // repo2 has backend AND api, not frontend } + + #[test] + fn test_save_config_preserves_comments() { + use std::io::Write; + let temp_dir = std::env::temp_dir(); + let config_path = temp_dir.join("test_config_comments.yaml"); + + // Create a config file with leading comments + let content = r#"# This is a comment +# Another comment line +--- + repositories: + - name: test-repo + url: https://github.com/test/repo + tags: + - test +"#; + let mut file = std::fs::File::create(&config_path).unwrap(); + file.write_all(content.as_bytes()).unwrap(); + drop(file); + + // Load the config + let mut config = Config::load(config_path.to_str().unwrap()).unwrap(); + + // Modify the config + config.repositories[0].add_tag("new-tag".to_string()); + + // Save it back + config.save(config_path.to_str().unwrap()).unwrap(); + + // Read the file and verify comments are preserved + let saved_content = std::fs::read_to_string(&config_path).unwrap(); + + assert!(saved_content.contains("# This is a comment")); + assert!(saved_content.contains("# Another comment line")); + assert!(saved_content.contains("---")); + assert!(saved_content.contains("new-tag")); + + // Cleanup + std::fs::remove_file(&config_path).unwrap(); + } + + #[test] + fn test_save_config_without_existing_file() { + let temp_dir = std::env::temp_dir(); + let config_path = temp_dir.join("test_config_new.yaml"); + + // Ensure file doesn't exist + let _ = std::fs::remove_file(&config_path); + + // Create and save a new config + let mut config = Config::new(); + let mut repo = Repository::new( + "new-repo".to_string(), + "https://github.com/test/new".to_string(), + ); + repo.add_tag("tag1".to_string()); + config.repositories.push(repo); + + config.save(config_path.to_str().unwrap()).unwrap(); + + // Read and verify + let content = std::fs::read_to_string(&config_path).unwrap(); + + assert!(content.starts_with("---\n")); + assert!(content.contains("new-repo")); + assert!(content.contains("tag1")); + assert!(content.ends_with('\n')); + + // Cleanup + std::fs::remove_file(&config_path).unwrap(); + } } diff --git a/src/lib.rs b/src/lib.rs index 125300b..1d34e5d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,7 @@ pub type Result = anyhow::Result; // Re-export commonly used types pub use commands::{Command, CommandContext}; +pub use config::loader::save_config; pub use config::{Config, Repository}; pub use github::PrOptions; pub use plugins::PluginContext;