refactor: add project_config#1539
Conversation
WalkthroughThe changes introduce a new struct called Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
crates/cli/src/config.rs (2)
57-60: Introduce#[derive(Debug)]forProjectConfigConsider adding
#[derive(Debug)]toProjectConfigto facilitate easier debugging and logging.Apply this change:
+#[derive(Debug)] pub struct ProjectConfig { pub project_dir: PathBuf, pub sg_config: AstGrepConfig, }
Line range hint
182-196: Handle absence of configuration file more gracefullyWhen no configuration file is found,
find_config_path_with_defaultdefaults to returningsgconfig.ymlin the current directory, which may not exist. This could lead to an error when trying to read the file. Consider explicitly notifying the user that no configuration file was found.You can adjust the function to return an error if the configuration file doesn't exist:
} else { - break Ok(PathBuf::from(CONFIG_FILE)); + break Err(anyhow::anyhow!("No configuration file found")); }This change provides clear feedback when a configuration file is missing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- crates/cli/src/config.rs (4 hunks)
🧰 Additional context used
🔇 Additional comments (4)
crates/cli/src/config.rs (4)
86-88: Refactor enhances code readabilityRefactoring to use
ProjectConfigimproves code clarity by encapsulating configuration details.
110-113: Accept references to avoid unnecessary cloningUpdating
build_util_walkerto accept references prevents unnecessary cloning ofutil_dirs, improving performance.
120-125: Simplify access to configuration with&ProjectConfigRefactoring
find_util_rulesto accept&ProjectConfigstreamlines access to configuration data and enhances maintainability.
151-160: Enhance function by accepting&ProjectConfigModifying
read_directory_yamlto take&ProjectConfigsimplifies parameter passing and improves code cohesion.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1539 +/- ##
==========================================
+ Coverage 72.24% 72.31% +0.07%
==========================================
Files 88 88
Lines 6009 6018 +9
==========================================
+ Hits 4341 4352 +11
+ Misses 1668 1666 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
crates/cli/src/config.rs (1)
Line range hint
113-118: Simplifybuild_util_walkerparameter handlingConsider changing the
util_dirsparameter to a slice to avoid unnecessaryOptionwrapping and make the function more idiomatic.Apply this diff:
-fn build_util_walker(base_dir: &Path, util_dirs: &Option<Vec<PathBuf>>) -> Option<WalkBuilder> { - let mut util_dirs = util_dirs.as_ref()?.iter(); +fn build_util_walker(base_dir: &Path, util_dirs: &[PathBuf]) -> Option<WalkBuilder> { + if util_dirs.is_empty() { + return None; + } + let mut util_dirs = util_dirs.iter();Update the call sites accordingly:
-let Some(mut walker) = build_util_walker(project_dir, &sg_config.util_dirs) else { +let Some(mut walker) = build_util_walker(project_dir, sg_config.util_dirs.as_deref().unwrap_or(&[])) else { return Ok(GlobalRules::default()); };crates/cli/src/new.rs (1)
Line range hint
321-338: Refactor duplicated directory selection logicThe logic for selecting the
util_dirincreate_new_utilis duplicated fromcreate_new_rule. To adhere to the DRY principle and improve maintainability, consider utilizing the same helper function suggested earlier.Update
create_new_utilusing theselect_directoryhelper function:-let util_dir = if utils.len() > 1 { - let dirs = utils.iter().map(|p| p.display()).collect(); - let display = - inquire::Select::new("Which util dir do you want to save your rule?", dirs).prompt()?; - project_dir.join(display.to_string()) -} else { - project_dir.join(&utils[0]) -}; +let util_dir = select_directory( + "Which util dir do you want to save your util?", + &utils, + &project_dir, + arg.yes, +)?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- crates/cli/src/config.rs (5 hunks)
- crates/cli/src/new.rs (6 hunks)
🧰 Additional context used
🔇 Additional comments (13)
crates/cli/src/config.rs (6)
57-60: Good introduction ofProjectConfigstructIntroducing the
ProjectConfigstruct improves configuration management by encapsulating related data, enhancing code readability and maintainability.
62-83: Well-implementedProjectConfigmethodsThe methods
by_config_path,by_project_dir, andfindprovide convenient ways to initialize project configurations, making the codebase more cohesive.
89-91: Effective use ofProjectConfiginfind_rulesRefactoring to use
ProjectConfiginfind_rulesenhances consistency in configuration handling.
123-128: Effective utilization ofProjectConfiginfind_util_rulesPassing
&ProjectConfigsimplifies function parameters and enhances code clarity.
154-164: Consistent use ofProjectConfiginread_directory_yamlUsing
ProjectConfigensures consistency when accessing configuration data, improving maintainability.
229-230: Simplifiedread_config_from_dirfunctionUpdating
read_config_from_dirto returnProjectConfigstreamlines configuration retrieval and aligns with the new structure.crates/cli/src/new.rs (7)
161-161: Updated usage ofread_config_from_dirwithProjectConfigThe function
run_create_entitynow correctly handles theProjectConfigreturned byread_config_from_dir, ensuring that the project configuration is appropriately utilized.
173-177: Refactoreddo_create_entityto acceptProjectConfigThe function signature of
do_create_entityhas been updated to acceptProjectConfig, and the subsequent match arms have been adjusted accordingly. The changes correctly adapt the function to the new configuration structure.
185-185: Consistent handling ofProjectConfiginask_entity_typeThe function
ask_entity_typenow properly usesread_config_from_dirto obtain aProjectConfigand proceeds accordingly based on the result, maintaining consistency with the updated configuration handling.
242-246: Updatedcreate_new_ruleto utilizeProjectConfigThe function now accepts
ProjectConfigand destructures it intoproject_dirandsg_config, allowing for clear access to project-related configurations.
252-254: Correct path construction forrule_dirThe code correctly joins
project_dirwith the selected or defaultrule_dirto determine where to save the new rule, ensuring that the rule is placed in the appropriate directory.
321-325: Adaptedcreate_new_utilto acceptProjectConfigThe function
create_new_utilnow acceptsProjectConfigand destructures it to accessproject_dirandsg_config, aligning it with the updated configuration structure.
336-338: Correct path construction forutil_dirThe code correctly joins
project_dirwith the selected or defaultutil_dirto determine where to save the new utility, ensuring that the utility is placed in the appropriate directory.
Summary by CodeRabbit
New Features
ProjectConfigstructure to enhance configuration management.ProjectConfig, simplifying access to configurations.Bug Fixes