Skip to content

Use preset path directly if it exists #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xarantolus merged 2 commits into from
Apr 6, 2026
Merged

Use preset path directly if it exists #44

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e05670
Use preset path directly if it exists
xarantolus Apr 1, 2026
6a08303
Update preset argument documentation
xarantolus Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions xtasks/crates/config/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct Cli {
enum Subcommand {
/// Load a preset configuration.
Load {
/// Name of the preset to load. Must correspond to a file in the `presets/` directory without the `.toml` extension.
/// Name of the preset to load. Can either be a path, or the name of a file in the `presets/` directory without the `.toml` extension.
preset: String,
/// Do not ask for confirmation.
#[arg(long, default_value_t = false)]
Expand Down Expand Up @@ -79,8 +79,14 @@ fn ask_confirmation(prompt: &str) -> bool {
}

fn run_load_preset(preset_name: &str, no_confirm: bool, current_dir: &Path) -> Result<(), Error> {
// Load the preset file from the `presets/` directory.
let preset_path = PathBuf::from("presets").join(format!("{preset_name}.toml"));
// Load the file, or get it from the `presets/` directory
let pb = PathBuf::from(preset_name);
let preset_path = if pb.is_file() {
pb
} else {
PathBuf::from("presets").join(format!("{preset_name}.toml"))
Comment thread
xarantolus marked this conversation as resolved.
Comment on lines +82 to +87
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The Load.preset CLI help text still says the argument "must correspond to a file in the presets/ directory without the .toml extension", but this function now accepts direct file paths as well. Update the clap docstring (and any related docs) so users discover the new behavior and don't get misled about what values are valid.

Copilot uses AI. Check for mistakes.
};
Comment on lines +83 to +88
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

pb.is_file() (and the fallback PathBuf::from("presets")...) resolve paths relative to the process working directory, not the current_dir/--root passed into this function. Since just config invokes the xtask with --root {{justfile_directory()}}, running just from a different directory can cause existing preset paths to be missed and the wrong fallback path to be used. Consider resolving relative preset_names (and the presets/ directory) against current_dir to make --root consistently control where presets are found.

Suggested change
let pb = PathBuf::from(preset_name);
let preset_path = if pb.is_file() {
pb
} else {
PathBuf::from("presets").join(format!("{preset_name}.toml"))
};
// Resolve the preset path relative to `current_dir` (the `--root`), unless `preset_name` is absolute.
let candidate = if Path::new(preset_name).is_absolute() {
PathBuf::from(preset_name)
} else {
current_dir.join(preset_name)
};
let preset_path = if candidate.is_file() {
candidate
} else {
current_dir
.join("presets")
.join(format!("{preset_name}.toml"))
};

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@xarantolus xarantolus Apr 6, 2026

Choose a reason for hiding this comment

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

Imo in this case it makes sense to do this relative? As user I just expect to pass a path and have that read


let preset = config::load_toml(&preset_path)?;

let config_path = current_dir.join("config.toml");
Expand Down
Loading