The current implementation of the run function in the CLI module is becoming complex due to the inlining of multiple steps, especially within the Build command arm. This complexity makes the function harder to read and maintain.
Suggested Refactor:
- Extract the key steps in the
Build command into small, well-named helper functions. For example:
handle_build(cli, targets) to encapsulate the build process
resolve_manifest_path(cli) to handle manifest path resolution
write_ninja_file(cli, graph) to handle writing the Ninja build file
- This approach will flatten out the
Build arm, remove nested map_or_else calls, and keep the run function shallow and easy to follow.
- All existing functionality should be preserved.
Example Refactored Code:
pub fn run(cli: &Cli) -> io::Result<()> {
match cli.command.clone().unwrap_or_default() {
Commands::Build { targets } => handle_build(cli, &targets),
Commands::Clean => { println!("Clean requested"); Ok(()) }
Commands::Graph => { println!("Graph requested"); Ok(()) }
}
}
fn handle_build(cli: &Cli, targets: &[String]) -> io::Result<()> {
let manifest_path = resolve_manifest_path(cli);
let manifest = manifest::from_path(&manifest_path).map_err(io::Error::other)?;
debug!("AST:\n{}", serde_json::to_string_pretty(&manifest).unwrap());
let graph = BuildGraph::from_manifest(&manifest).map_err(io::Error::other)?;
let ninja_path = write_ninja_file(cli, &graph)?;
info!("Generated Ninja file at {}", ninja_path.display());
run_ninja(Path::new("ninja"), cli, targets)
}
fn resolve_manifest_path(cli: &Cli) -> PathBuf {
cli.directory
.as_ref()
.map(|dir| dir.join(&cli.file))
.unwrap_or_else(|| cli.file.clone())
}
fn write_ninja_file(cli: &Cli, graph: &BuildGraph) -> io::Result<PathBuf> {
let content = ninja_gen::generate(graph);
let path = cli
.directory
.as_ref()
.map(|d| d.join("build.ninja"))
.unwrap_or_else(|| PathBuf::from("build.ninja"));
fs::write(&path, content)
.map(|_| path)
.map_err(io::Error::other)
}
Action Items:
- Refactor the
run function as described above.
- Ensure all tests pass and functionality is preserved.
- Review for further simplification opportunities if possible.
References
I created this issue for @leynos from #45 (comment).
Tips and commands
Getting Help
The current implementation of the
runfunction in the CLI module is becoming complex due to the inlining of multiple steps, especially within theBuildcommand arm. This complexity makes the function harder to read and maintain.Suggested Refactor:
Buildcommand into small, well-named helper functions. For example:handle_build(cli, targets)to encapsulate the build processresolve_manifest_path(cli)to handle manifest path resolutionwrite_ninja_file(cli, graph)to handle writing the Ninja build fileBuildarm, remove nestedmap_or_elsecalls, and keep therunfunction shallow and easy to follow.Example Refactored Code:
Action Items:
runfunction as described above.References
runfunction to reduce complexity by extracting helper functions #47I created this issue for @leynos from #45 (comment).
Tips and commands
Getting Help