Skip to content

Conversation

@itschip
Copy link
Member

@itschip itschip commented May 24, 2025

Summary

This PR implements automatic resources directory detection and validation for both init and install commands, ensuring that modules.json is always created in the correct location.

Key Changes

  • Added find_resources_directory() function that traverses upward to find resources directory
  • Updated both init and install commands to validate they operate within resources directory
  • Enhanced error handling with descriptive messages when resources directory not found
  • Improved user feedback with clear success/status messages

How It Works

The commands now work from:

  • ✅ Within resources directory
  • ✅ Subdirectories of resources (traverses upward)
  • ✅ Parent directories containing resources (finds subdirectory)
  • ❌ Unrelated directories (shows error and exits)

This ensures modules.json is always created in the correct resources directory regardless of where the user runs the command.

@itschip
Copy link
Member Author

itschip commented May 24, 2025

this is an mcp test

@itschip itschip requested a review from Copilot May 24, 2025 20:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds automatic detection and validation of the resources directory for the init and install commands, ensuring that modules.json is always placed under the correct directory.

  • Introduces find_resources_directory() to locate the nearest resources folder
  • Updates CLI flow in main.rs to call validation before running init or install
  • Adds user feedback prints in installer::init_fxpkg

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/main.rs New helper to find resources dir and updated command logic
src/installer/mod.rs Added success message print after creating modules.json
Comments suppressed due to low confidence (1)

src/main.rs:71

  • No tests cover the failure path of find_resources_directory; consider adding unit tests to verify both success and error scenarios.
Err("Could not find 'resources' directory. Please run 'fxpkg init' from within or above a 'resources' directory.".to_string())

Comment on lines +48 to +60
fn find_resources_directory(current_path: &Path) -> Result<String, String> {
let mut path = current_path.to_path_buf();

// Check if current directory is already "resources"
if path.file_name().and_then(|n| n.to_str()) == Some("resources") {
return Ok(path.to_string_lossy().to_string());
}

// Traverse upwards to find "resources" directory
loop {
let resources_path = path.join("resources");
if resources_path.exists() && resources_path.is_dir() {
return Ok(resources_path.to_string_lossy().to_string());
Copy link

Copilot AI May 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning a PathBuf instead of String to preserve path semantics and avoid unnecessary string conversions.

Suggested change
fn find_resources_directory(current_path: &Path) -> Result<String, String> {
let mut path = current_path.to_path_buf();
// Check if current directory is already "resources"
if path.file_name().and_then(|n| n.to_str()) == Some("resources") {
return Ok(path.to_string_lossy().to_string());
}
// Traverse upwards to find "resources" directory
loop {
let resources_path = path.join("resources");
if resources_path.exists() && resources_path.is_dir() {
return Ok(resources_path.to_string_lossy().to_string());
fn find_resources_directory(current_path: &Path) -> Result<PathBuf, String> {
let mut path = current_path.to_path_buf();
// Check if current directory is already "resources"
if path.file_name().and_then(|n| n.to_str()) == Some("resources") {
return Ok(path);
}
// Traverse upwards to find "resources" directory
loop {
let resources_path = path.join("resources");
if resources_path.exists() && resources_path.is_dir() {
return Ok(resources_path);

Copilot uses AI. Check for mistakes.
/// The package to install
name: String,
}

Copy link

Copilot AI May 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a doc comment explaining the lookup strategy and error cases for find_resources_directory to improve readability and maintainability.

Suggested change
/// Locates the "resources" directory starting from the given path.
///
/// This function first checks if the current directory is named "resources".
/// If not, it traverses upwards through parent directories to find a directory
/// named "resources". If the directory is found, its path is returned as a `String`.
///
/// # Errors
/// Returns an `Err` with a descriptive message if the "resources" directory
/// cannot be found. This typically happens if the function is called from a
/// location that is not within or above a "resources" directory.

Copilot uses AI. Check for mistakes.
Comment on lines +83 to +106
match find_resources_directory(&pwd) {
Ok(resources_path) => {
let mut pkg_installer = installer::PackageInstall::new();
pkg_installer
.install(&resources_path, &package.name)
.await;
}
Err(error_msg) => {
eprintln!("Error: {}", error_msg);
std::process::exit(1);
}
}
}
Commands::Init => {
let pwd = env::current_dir().unwrap();
let path = pwd.to_str().unwrap();

installer::init_fxpkg(path);
match find_resources_directory(&pwd) {
Ok(resources_path) => {
installer::init_fxpkg(&resources_path);
println!("Initialized fxpkg in: {}", resources_path);
}
Err(error_msg) => {
eprintln!("Error: {}", error_msg);
std::process::exit(1);
}
}
Copy link

Copilot AI May 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic for both commands is duplicated; extract the match block into a helper to reduce repetition.

Copilot uses AI. Check for mistakes.
@itschip
Copy link
Member Author

itschip commented May 24, 2025

mcp test

@itschip itschip closed this May 24, 2025
@itschip itschip deleted the feature/resources-directory-validation branch May 24, 2025 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants