Skip to content
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.

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

config: remove check for relative path patterns, add doc about path equivalence #5145

Merged
merged 1 commit into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -1256,4 +1256,13 @@ Condition keys:

* `--when.repositories`: List of paths to match the repository path prefix.

Paths should be absolute. Each path component (directory or file name, drive
letter, etc.) is compared case-sensitively on all platforms. A path starting
with `~` is expanded to the home directory. On Windows, directory separator may
be either `\` or `/`. (Beware that `\` needs escape in double-quoted strings.)

Use `jj root` to see the workspace root directory. Note that the repository path
is in the main workspace if you're using multiple workspaces with `jj
workspace`.

If no conditions are specified, table is always enabled.
19 changes: 3 additions & 16 deletions lib/src/config_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,13 @@ impl ScopeCondition {
}

fn expand_paths(mut self, context: &ConfigResolutionContext) -> Result<Self, &'static str> {
// It might make some sense to compare paths in canonicalized form, but
// be careful to not resolve relative path patterns against cwd, which
// wouldn't be what the user would expect.
for path in self.repositories.as_mut().into_iter().flatten() {
// TODO: might be better to compare paths in canonicalized form?
if let Some(new_path) = expand_home(path, context.home_dir)? {
*path = new_path;
}
// TODO: better to skip relative paths instead of an error? The rule
// differs across platforms. "/" is relative path on Windows.
// Another option is to add "platforms = [..]" predicate and do path
// validation lazily.
if path.is_relative() {
return Err("Relative path in --when.repositories");
}
}
Ok(self)
}
Expand Down Expand Up @@ -344,7 +339,6 @@ mod tests {
}

#[test]
#[cfg(not(windows))] // '/' is not absolute path on Windows
fn test_resolve_repo_path() {
let mut source_config = StackedConfig::empty();
source_config.add_layer(new_user_layer(indoc! {"
Expand Down Expand Up @@ -419,13 +413,6 @@ mod tests {
resolve(&new_config("--when.repositories = 0"), &context),
Err(ConfigGetError::Type { .. })
);
assert_matches!(
resolve(
&new_config("--when.repositories = ['relative/path']"),
&context
),
Err(ConfigGetError::Type { .. })
);
}

#[test]
Expand Down
Loading