diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index a81cae8f7..4761ff4ea 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -36,7 +36,7 @@ use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; use jj_lib::backend::{ChangeId, CommitId, MergedTreeId, TreeValue}; use jj_lib::commit::Commit; -use jj_lib::fileset::{FilesetExpression, FilesetParseContext}; +use jj_lib::fileset::FilesetExpression; use jj_lib::git_backend::GitBackend; use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile}; use jj_lib::hex_util::to_reverse_hex; @@ -755,19 +755,15 @@ impl WorkspaceCommandHelper { &self, file_args: &[String], // TODO: introduce FileArg newtype? ) -> Result { - let ctx = self.fileset_parse_context(); let expressions: Vec<_> = file_args .iter() - .map(|arg| fileset::parse_maybe_bare(arg, &ctx)) + .map(|arg| fileset::parse_maybe_bare(arg, &self.path_converter)) .try_collect()?; Ok(FilesetExpression::union_all(expressions)) } - pub(crate) fn fileset_parse_context(&self) -> FilesetParseContext<'_> { - FilesetParseContext { - cwd: &self.cwd, - workspace_root: self.workspace.workspace_root(), - } + pub(crate) fn path_converter(&self) -> &RepoPathUiConverter { + &self.path_converter } #[instrument(skip_all)] diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index b179f152f..cf4b570e5 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -553,9 +553,11 @@ fn file_pattern_parse_error_hint(err: &FilePatternParseError) -> Option match err { FilePatternParseError::InvalidKind(_) => None, // Suggest root:"" if input can be parsed as repo-relative path - FilePatternParseError::FsPath(e) => RepoPathBuf::from_relative_path(&e.input) - .ok() - .map(|path| format!(r#"Consider using root:{path:?} to specify repo-relative path"#)), + FilePatternParseError::UiPath(UiPathParseError::Fs(e)) => { + RepoPathBuf::from_relative_path(&e.input).ok().map(|path| { + format!(r#"Consider using root:{path:?} to specify repo-relative path"#) + }) + } FilePatternParseError::RelativePath(_) => None, FilePatternParseError::GlobPattern(_) => None, } diff --git a/cli/src/commands/debug.rs b/cli/src/commands/debug.rs index a3f3d24bc..79401c0cb 100644 --- a/cli/src/commands/debug.rs +++ b/cli/src/commands/debug.rs @@ -149,9 +149,9 @@ fn cmd_debug_fileset( args: &DebugFilesetArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; - let ctx = workspace_command.fileset_parse_context(); + let path_converter = workspace_command.path_converter(); - let expression = fileset::parse_maybe_bare(&args.path, &ctx)?; + let expression = fileset::parse_maybe_bare(&args.path, path_converter)?; writeln!(ui.stdout(), "-- Parsed:")?; writeln!(ui.stdout(), "{expression:#?}")?; writeln!(ui.stdout())?; diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs index f6e35822c..a90f9eed8 100644 --- a/lib/src/fileset.rs +++ b/lib/src/fileset.rs @@ -15,7 +15,6 @@ //! Functional language for selecting a set of paths. use std::collections::HashMap; -use std::path::Path; use std::{iter, path, slice}; use once_cell::sync::Lazy; @@ -30,7 +29,9 @@ use crate::matchers::{ DifferenceMatcher, EverythingMatcher, FileGlobsMatcher, FilesMatcher, IntersectionMatcher, Matcher, NothingMatcher, PrefixMatcher, UnionMatcher, }; -use crate::repo_path::{FsPathParseError, RelativePathParseError, RepoPath, RepoPathBuf}; +use crate::repo_path::{ + RelativePathParseError, RepoPath, RepoPathBuf, RepoPathUiConverter, UiPathParseError, +}; /// Error occurred during file pattern parsing. #[derive(Debug, Error)] @@ -38,9 +39,9 @@ pub enum FilePatternParseError { /// Unknown pattern kind is specified. #[error(r#"Invalid file pattern kind "{0}:""#)] InvalidKind(String), - /// Failed to parse input cwd-relative path. + /// Failed to parse input UI path. #[error(transparent)] - FsPath(#[from] FsPathParseError), + UiPath(#[from] UiPathParseError), /// Failed to parse input workspace-relative path. #[error(transparent)] RelativePath(#[from] RelativePathParseError), @@ -71,7 +72,7 @@ pub enum FilePattern { impl FilePattern { /// Parses the given `input` string as pattern of the specified `kind`. pub fn from_str_kind( - ctx: &FilesetParseContext, + path_converter: &RepoPathUiConverter, input: &str, kind: &str, ) -> Result { @@ -90,9 +91,9 @@ impl FilePattern { // * glob: glob pattern (default anchor: file) // * regex? match kind { - "cwd" => Self::cwd_prefix_path(ctx, input), - "cwd-file" | "file" => Self::cwd_file_path(ctx, input), - "cwd-glob" | "glob" => Self::cwd_file_glob(ctx, input), + "cwd" => Self::cwd_prefix_path(path_converter, input), + "cwd-file" | "file" => Self::cwd_file_path(path_converter, input), + "cwd-glob" | "glob" => Self::cwd_file_glob(path_converter, input), "root" => Self::root_prefix_path(input), "root-file" => Self::root_file_path(input), "root-glob" => Self::root_file_glob(input), @@ -102,41 +103,42 @@ impl FilePattern { /// Pattern that matches cwd-relative file (or exact) path. pub fn cwd_file_path( - ctx: &FilesetParseContext, - input: impl AsRef, + path_converter: &RepoPathUiConverter, + input: impl AsRef, ) -> Result { - let path = ctx.parse_cwd_path(input)?; + let path = path_converter.parse_file_path(input.as_ref())?; Ok(FilePattern::FilePath(path)) } /// Pattern that matches cwd-relative path prefix. pub fn cwd_prefix_path( - ctx: &FilesetParseContext, - input: impl AsRef, + path_converter: &RepoPathUiConverter, + input: impl AsRef, ) -> Result { - let path = ctx.parse_cwd_path(input)?; + let path = path_converter.parse_file_path(input.as_ref())?; Ok(FilePattern::PrefixPath(path)) } /// Pattern that matches cwd-relative file path glob. pub fn cwd_file_glob( - ctx: &FilesetParseContext, + path_converter: &RepoPathUiConverter, input: impl AsRef, ) -> Result { let (dir, pattern) = split_glob_path(input.as_ref()); - let dir = ctx.parse_cwd_path(dir)?; + let dir = path_converter.parse_file_path(dir)?; Self::file_glob_at(dir, pattern) } /// Pattern that matches workspace-relative file (or exact) path. - pub fn root_file_path(input: impl AsRef) -> Result { - let path = RepoPathBuf::from_relative_path(input)?; + pub fn root_file_path(input: impl AsRef) -> Result { + // TODO: Let caller pass in converter for root-relative paths too + let path = RepoPathBuf::from_relative_path(input.as_ref())?; Ok(FilePattern::FilePath(path)) } /// Pattern that matches workspace-relative path prefix. - pub fn root_prefix_path(input: impl AsRef) -> Result { - let path = RepoPathBuf::from_relative_path(input)?; + pub fn root_prefix_path(input: impl AsRef) -> Result { + let path = RepoPathBuf::from_relative_path(input.as_ref())?; Ok(FilePattern::PrefixPath(path)) } @@ -373,33 +375,18 @@ fn union_all_matchers(matchers: &mut [Option>]) -> Box { - /// Normalized path to the current working directory. - pub cwd: &'a Path, - /// Normalized path to the workspace root. - pub workspace_root: &'a Path, -} - -impl FilesetParseContext<'_> { - fn parse_cwd_path(&self, input: impl AsRef) -> Result { - RepoPathBuf::parse_fs_path(self.cwd, self.workspace_root, input) - } -} - type FilesetFunction = - fn(&FilesetParseContext, &FunctionCallNode) -> FilesetParseResult; + fn(&RepoPathUiConverter, &FunctionCallNode) -> FilesetParseResult; static BUILTIN_FUNCTION_MAP: Lazy> = Lazy::new(|| { // Not using maplit::hashmap!{} or custom declarative macro here because // code completion inside macro is quite restricted. let mut map: HashMap<&'static str, FilesetFunction> = HashMap::new(); - map.insert("none", |_ctx, function| { + map.insert("none", |_path_converter, function| { function.expect_no_arguments()?; Ok(FilesetExpression::none()) }); - map.insert("all", |_ctx, function| { + map.insert("all", |_path_converter, function| { function.expect_no_arguments()?; Ok(FilesetExpression::all()) }); @@ -407,11 +394,11 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy }); fn resolve_function( - ctx: &FilesetParseContext, + path_converter: &RepoPathUiConverter, function: &FunctionCallNode, ) -> FilesetParseResult { if let Some(func) = BUILTIN_FUNCTION_MAP.get(function.name) { - func(ctx, function) + func(path_converter, function) } else { Err(FilesetParseError::new( FilesetParseErrorKind::NoSuchFunction { @@ -424,41 +411,43 @@ fn resolve_function( } fn resolve_expression( - ctx: &FilesetParseContext, + path_converter: &RepoPathUiConverter, node: &ExpressionNode, ) -> FilesetParseResult { let wrap_pattern_error = |err| FilesetParseError::expression("Invalid file pattern", node.span).with_source(err); match &node.kind { ExpressionKind::Identifier(name) => { - let pattern = FilePattern::cwd_prefix_path(ctx, name).map_err(wrap_pattern_error)?; + let pattern = + FilePattern::cwd_prefix_path(path_converter, name).map_err(wrap_pattern_error)?; Ok(FilesetExpression::pattern(pattern)) } ExpressionKind::String(name) => { - let pattern = FilePattern::cwd_prefix_path(ctx, name).map_err(wrap_pattern_error)?; + let pattern = + FilePattern::cwd_prefix_path(path_converter, name).map_err(wrap_pattern_error)?; Ok(FilesetExpression::pattern(pattern)) } ExpressionKind::StringPattern { kind, value } => { - let pattern = - FilePattern::from_str_kind(ctx, value, kind).map_err(wrap_pattern_error)?; + let pattern = FilePattern::from_str_kind(path_converter, value, kind) + .map_err(wrap_pattern_error)?; Ok(FilesetExpression::pattern(pattern)) } ExpressionKind::Unary(op, arg_node) => { - let arg = resolve_expression(ctx, arg_node)?; + let arg = resolve_expression(path_converter, arg_node)?; match op { UnaryOp::Negate => Ok(FilesetExpression::all().difference(arg)), } } ExpressionKind::Binary(op, lhs_node, rhs_node) => { - let lhs = resolve_expression(ctx, lhs_node)?; - let rhs = resolve_expression(ctx, rhs_node)?; + let lhs = resolve_expression(path_converter, lhs_node)?; + let rhs = resolve_expression(path_converter, rhs_node)?; match op { BinaryOp::Union => Ok(lhs.union(rhs)), BinaryOp::Intersection => Ok(lhs.intersection(rhs)), BinaryOp::Difference => Ok(lhs.difference(rhs)), } } - ExpressionKind::FunctionCall(function) => resolve_function(ctx, function), + ExpressionKind::FunctionCall(function) => resolve_function(path_converter, function), } } @@ -468,15 +457,17 @@ fn resolve_expression( /// contain any operator-like characters, it will be parsed as a file path. pub fn parse_maybe_bare( text: &str, - ctx: &FilesetParseContext, + path_converter: &RepoPathUiConverter, ) -> FilesetParseResult { let node = fileset_parser::parse_program_or_bare_string(text)?; // TODO: add basic tree substitution pass to eliminate redundant expressions - resolve_expression(ctx, &node) + resolve_expression(path_converter, &node) } #[cfg(test)] mod tests { + use std::path::PathBuf; + use super::*; fn repo_path_buf(value: impl Into) -> RepoPathBuf { @@ -485,11 +476,11 @@ mod tests { #[test] fn test_parse_file_pattern() { - let ctx = FilesetParseContext { - cwd: Path::new("/ws/cur"), - workspace_root: Path::new("/ws"), + let path_converter = RepoPathUiConverter::Fs { + cwd: PathBuf::from("/ws/cur"), + base: PathBuf::from("/ws"), }; - let parse = |text| parse_maybe_bare(text, &ctx); + let parse = |text| parse_maybe_bare(text, &path_converter); // cwd-relative patterns assert_eq!( @@ -536,12 +527,12 @@ mod tests { #[test] fn test_parse_glob_pattern() { - let ctx = FilesetParseContext { + let path_converter = RepoPathUiConverter::Fs { // meta character in cwd path shouldn't be expanded - cwd: Path::new("/ws/cur*"), - workspace_root: Path::new("/ws"), + cwd: PathBuf::from("/ws/cur*"), + base: PathBuf::from("/ws"), }; - let parse = |text| parse_maybe_bare(text, &ctx); + let parse = |text| parse_maybe_bare(text, &path_converter); let glob_expr = |dir: &str, pattern: &str| { FilesetExpression::pattern(FilePattern::FileGlob { dir: repo_path_buf(dir), @@ -620,11 +611,11 @@ mod tests { #[test] fn test_parse_function() { - let ctx = FilesetParseContext { - cwd: Path::new("/ws/cur"), - workspace_root: Path::new("/ws"), + let path_converter = RepoPathUiConverter::Fs { + cwd: PathBuf::from("/ws/cur"), + base: PathBuf::from("/ws"), }; - let parse = |text| parse_maybe_bare(text, &ctx); + let parse = |text| parse_maybe_bare(text, &path_converter); assert_eq!(parse("all()").unwrap(), FilesetExpression::all()); assert_eq!(parse("none()").unwrap(), FilesetExpression::none()); @@ -646,11 +637,11 @@ mod tests { #[test] fn test_parse_compound_expression() { - let ctx = FilesetParseContext { - cwd: Path::new("/ws/cur"), - workspace_root: Path::new("/ws"), + let path_converter = RepoPathUiConverter::Fs { + cwd: PathBuf::from("/ws/cur"), + base: PathBuf::from("/ws"), }; - let parse = |text| parse_maybe_bare(text, &ctx); + let parse = |text| parse_maybe_bare(text, &path_converter); insta::assert_debug_snapshot!(parse("~x").unwrap(), @r###" Difference( diff --git a/lib/src/revset.rs b/lib/src/revset.rs index cb06cc575..d092159f6 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -32,13 +32,14 @@ use thiserror::Error; use crate::backend::{BackendError, BackendResult, ChangeId, CommitId}; use crate::commit::Commit; use crate::dsl_util::{collect_similar, AliasId}; -use crate::fileset::{FilePattern, FilesetExpression, FilesetParseContext}; +use crate::fileset::{FilePattern, FilesetExpression}; use crate::git; use crate::hex_util::to_forward_hex; use crate::id_prefix::IdPrefixContext; use crate::object_id::{HexPrefix, PrefixResolution}; use crate::op_store::WorkspaceId; use crate::repo::Repo; +use crate::repo_path::RepoPathUiConverter; use crate::revset_graph::RevsetGraphEdge; // TODO: introduce AST types and remove parse_expression_rule, Rule from the // re-exports @@ -782,13 +783,15 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: map.insert("file", |name, arguments_pair, state| { let arguments_span = arguments_pair.as_span(); if let Some(ctx) = state.workspace_ctx { - let ctx = FilesetParseContext { - cwd: ctx.cwd, - workspace_root: ctx.workspace_root, + let path_converter = RepoPathUiConverter::Fs { + cwd: ctx.cwd.to_owned(), + base: ctx.workspace_root.to_owned(), }; let file_expressions: Vec<_> = arguments_pair .into_inner() - .map(|arg| parse_function_argument_to_file_pattern(name, arg, state, &ctx)) + .map(|arg| { + parse_function_argument_to_file_pattern(name, arg, state, &path_converter) + }) .map_ok(FilesetExpression::pattern) .try_collect()?; if file_expressions.is_empty() { @@ -824,11 +827,11 @@ pub fn parse_function_argument_to_file_pattern( name: &str, pair: Pair, state: ParseState, - ctx: &FilesetParseContext, + path_converter: &RepoPathUiConverter, ) -> Result { let parse_pattern = |value: &str, kind: Option<&str>| match kind { - Some(kind) => FilePattern::from_str_kind(ctx, value, kind), - None => FilePattern::cwd_prefix_path(ctx, value), + Some(kind) => FilePattern::from_str_kind(path_converter, value, kind), + None => FilePattern::cwd_prefix_path(path_converter, value), }; parse_function_argument_as_pattern("file pattern", name, pair, state, parse_pattern) }