Skip to content

Commit

Permalink
revset: add &FilesetAliasesMap to RevsetParseContext
Browse files Browse the repository at this point in the history
This propogates `[fileset-aliases]` to everywhere they're used. Chiefly,
they need to be accessible inside of expressions like:

    jj log -T 'self.diff("...").summary()'
    jj log -r 'diff_contains("words", "...")'
  • Loading branch information
bryceberger committed Nov 26, 2024
1 parent 6b1e2d3 commit 6e0f91d
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 26 deletions.
1 change: 1 addition & 0 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,7 @@ impl WorkspaceCommandEnvironment {
now.into(),
self.command.revset_extensions(),
Some(workspace_context),
&self.fileset_aliases_map,
)
}

Expand Down
12 changes: 9 additions & 3 deletions cli/src/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use tracing::instrument;

use crate::cli_util::CommandHelper;
use crate::cli_util::RevisionArg;
use crate::cli_util::WorkspaceCommandHelper;
use crate::command_error::config_error;
use crate::command_error::print_parse_diagnostics;
use crate::command_error::CommandError;
Expand Down Expand Up @@ -145,7 +146,7 @@ pub(crate) fn cmd_fix(
args: &FixArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let tools_config = get_tools_config(ui, command.settings())?;
let tools_config = get_tools_config(ui, &workspace_command, command.settings())?;
let root_commits: Vec<CommitId> = if args.source.is_empty() {
let revs = command.settings().get_string("revsets.fix")?;
workspace_command.parse_revset(ui, &RevisionArg::from(revs))?
Expand Down Expand Up @@ -446,7 +447,11 @@ struct RawToolConfig {
/// Fails if any of the commands or patterns are obviously unusable, but does
/// not check for issues that might still occur later like missing executables.
/// This is a place where we could fail earlier in some cases, though.
fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig, CommandError> {
fn get_tools_config(
ui: &mut Ui,
workspace_command: &WorkspaceCommandHelper,
settings: &UserSettings,
) -> Result<ToolsConfig, CommandError> {
let mut tools_config = ToolsConfig { tools: Vec::new() };
// TODO: Remove this block of code and associated documentation after at least
// one release where the feature is marked deprecated.
Expand Down Expand Up @@ -475,6 +480,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
}
if let Ok(tools_table) = settings.raw_config().get_table("fix.tools") {
// Convert the map into a sorted vector early so errors are deterministic.
let fileset_aliases_map = workspace_command.fileset_aliases_map();
let mut tools: Vec<ToolConfig> = tools_table
.into_iter()
.sorted_by(|a, b| a.0.cmp(&b.0))
Expand All @@ -492,7 +498,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
cwd: "".into(),
base: "".into(),
},
&Default::default(),
fileset_aliases_map,
)
})
.try_collect()?,
Expand Down
10 changes: 8 additions & 2 deletions cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use jj_lib::copies::CopiesTreeDiffEntry;
use jj_lib::copies::CopyRecords;
use jj_lib::extensions_map::ExtensionsMap;
use jj_lib::fileset;
use jj_lib::fileset::FilesetAliasesMap;
use jj_lib::fileset::FilesetDiagnostics;
use jj_lib::fileset::FilesetExpression;
use jj_lib::id_prefix::IdPrefixContext;
Expand Down Expand Up @@ -806,7 +807,12 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
|language, diagnostics, _build_ctx, self_property, function| {
let ([], [files_node]) = function.expect_arguments()?;
let files = if let Some(node) = files_node {
expect_fileset_literal(diagnostics, node, language.path_converter)?
expect_fileset_literal(
diagnostics,
node,
language.path_converter,
language.revset_parse_context.fileset_aliases_map(),
)?
} else {
// TODO: defaults to CLI path arguments?
// https://github.com/martinvonz/jj/issues/2933#issuecomment-1925870731
Expand Down Expand Up @@ -851,10 +857,10 @@ fn expect_fileset_literal(
diagnostics: &mut TemplateDiagnostics,
node: &ExpressionNode,
path_converter: &RepoPathUiConverter,
aliases_map: &FilesetAliasesMap,
) -> Result<FilesetExpression, TemplateParseError> {
template_parser::expect_string_literal_with(node, |text, span| {
let mut inner_diagnostics = FilesetDiagnostics::new();
let aliases_map = Default::default();
let expression = fileset::parse(&mut inner_diagnostics, text, path_converter, &aliases_map)
.map_err(|err| {
TemplateParseError::expression("In fileset expression", span).with_source(err)
Expand Down
2 changes: 1 addition & 1 deletion lib/src/fileset.pest
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,4 @@ program_or_bare_string = _{
| bare_string ~ EOI )
}

alias_declaration = _{ SOI ~ identifier ~ EOI }
alias_declaration = _{ SOI ~ strict_identifier ~ EOI }
4 changes: 3 additions & 1 deletion lib/src/fileset_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ pub fn parse_program_or_bare_string(text: &str) -> FilesetParseResult<Expression
Ok(ExpressionNode::new(expr, span))
}

/// User fileset aliases. When using the CLI, read from the "fileset-aliases"
/// table.
pub type FilesetAliasesMap = AliasesMap<FilesetAliasParser, String>;

#[derive(Clone, Debug, Default)]
Expand All @@ -439,7 +441,7 @@ impl AliasDeclarationParser for FilesetAliasParser {
let mut pairs = FilesetParser::parse(Rule::alias_declaration, source)?;
let first = pairs.next().unwrap();
match first.as_rule() {
Rule::identifier => Ok(AliasDeclaration::Symbol(first.as_str().to_owned())),
Rule::strict_identifier => Ok(AliasDeclaration::Symbol(first.as_str().to_owned())),
r => panic!("unexpected alias declaration rule {r:?}"),
}
}
Expand Down
60 changes: 43 additions & 17 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,12 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
}
let file_expressions = itertools::chain([arg], args)
.map(|arg| {
expect_fileset_expression(diagnostics, arg, ctx.path_converter, &Default::default())
expect_fileset_expression(
diagnostics,
arg,
ctx.path_converter,
context.fileset_aliases_map,
)
})
.try_collect()?;
let expr = FilesetExpression::union_all(file_expressions);
Expand All @@ -918,7 +923,7 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
diagnostics,
files_arg,
ctx.path_converter,
&Default::default(),
context.fileset_aliases_map,
)?
} else {
// TODO: defaults to CLI path arguments?
Expand Down Expand Up @@ -2647,6 +2652,7 @@ pub struct RevsetParseContext<'a> {
date_pattern_context: DatePatternContext,
extensions: &'a RevsetExtensions,
workspace: Option<RevsetWorkspaceContext<'a>>,
fileset_aliases_map: &'a FilesetAliasesMap,
}

impl<'a> RevsetParseContext<'a> {
Expand All @@ -2656,20 +2662,26 @@ impl<'a> RevsetParseContext<'a> {
date_pattern_context: DatePatternContext,
extensions: &'a RevsetExtensions,
workspace: Option<RevsetWorkspaceContext<'a>>,
fileset_aliases_map: &'a FilesetAliasesMap,
) -> Self {
Self {
aliases_map,
user_email,
date_pattern_context,
extensions,
workspace,
fileset_aliases_map,
}
}

pub fn aliases_map(&self) -> &'a RevsetAliasesMap {
self.aliases_map
}

pub fn fileset_aliases_map(&self) -> &'a FilesetAliasesMap {
self.fileset_aliases_map
}

pub fn user_email(&self) -> &str {
&self.user_email
}
Expand All @@ -2695,35 +2707,51 @@ mod tests {
use std::path::PathBuf;

use assert_matches::assert_matches;
use dsl_util::AliasesMap;

use super::*;

const EMPTY: [(&str, &str); 0] = [];

fn get_alias_map<T>(
aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
) -> AliasesMap<T, String>
where
T: dsl_util::AliasDeclarationParser + Default,
T::Error: std::fmt::Debug,
{
let mut aliases_map = AliasesMap::new();
for (decl, defn) in aliases {
aliases_map.insert(decl, defn).unwrap();
}
aliases_map
}

fn parse(revset_str: &str) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
parse_with_aliases(revset_str, [] as [(&str, &str); 0])
parse_with_aliases(revset_str, EMPTY)
}

fn parse_with_workspace(
revset_str: &str,
workspace_id: &WorkspaceId,
) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
parse_with_aliases_and_workspace(revset_str, [] as [(&str, &str); 0], workspace_id)
parse_with_aliases_and_workspace(revset_str, EMPTY, workspace_id)
}

fn parse_with_aliases(
revset_str: &str,
aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
let mut aliases_map = RevsetAliasesMap::new();
for (decl, defn) in aliases {
aliases_map.insert(decl, defn).unwrap();
}
let aliases_map = get_alias_map(aliases);
let fileset_aliases_map = get_alias_map(EMPTY);
let extensions = RevsetExtensions::default();
let context = RevsetParseContext::new(
&aliases_map,
"[email protected]".to_string(),
chrono::Utc::now().fixed_offset().into(),
&extensions,
None,
&fileset_aliases_map,
);
super::parse(&mut RevsetDiagnostics::new(), revset_str, &context)
}
Expand All @@ -2742,42 +2770,40 @@ mod tests {
path_converter: &path_converter,
workspace_id,
};
let mut aliases_map = RevsetAliasesMap::new();
for (decl, defn) in aliases {
aliases_map.insert(decl, defn).unwrap();
}
let aliases_map = get_alias_map(aliases);
let fileset_aliases_map = get_alias_map(EMPTY);
let extensions = RevsetExtensions::default();
let context = RevsetParseContext::new(
&aliases_map,
"[email protected]".to_string(),
chrono::Utc::now().fixed_offset().into(),
&extensions,
Some(workspace_ctx),
&fileset_aliases_map,
);
super::parse(&mut RevsetDiagnostics::new(), revset_str, &context)
}

fn parse_with_modifier(
revset_str: &str,
) -> Result<(Rc<UserRevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
parse_with_aliases_and_modifier(revset_str, [] as [(&str, &str); 0])
parse_with_aliases_and_modifier(revset_str, EMPTY)
}

fn parse_with_aliases_and_modifier(
revset_str: &str,
aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
) -> Result<(Rc<UserRevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
let mut aliases_map = RevsetAliasesMap::new();
for (decl, defn) in aliases {
aliases_map.insert(decl, defn).unwrap();
}
let aliases_map = get_alias_map(aliases);
let fileset_aliases_map = get_alias_map(EMPTY);
let extensions = RevsetExtensions::default();
let context = RevsetParseContext::new(
&aliases_map,
"[email protected]".to_string(),
chrono::Utc::now().fixed_offset().into(),
&extensions,
None,
&fileset_aliases_map,
);
super::parse_with_modifier(&mut RevsetDiagnostics::new(), revset_str, &context)
}
Expand Down
18 changes: 16 additions & 2 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use jj_lib::backend::MillisSinceEpoch;
use jj_lib::backend::Signature;
use jj_lib::backend::Timestamp;
use jj_lib::commit::Commit;
use jj_lib::fileset::FilesetAliasesMap;
use jj_lib::fileset::FilesetExpression;
use jj_lib::git;
use jj_lib::git_backend::GitBackend;
Expand Down Expand Up @@ -68,9 +69,16 @@ fn resolve_symbol_with_extensions(
symbol: &str,
) -> Result<Vec<CommitId>, RevsetResolutionError> {
let aliases_map = RevsetAliasesMap::default();
let fileset_aliases_map = FilesetAliasesMap::default();
let now = chrono::Local::now();
let context =
RevsetParseContext::new(&aliases_map, String::new(), now.into(), extensions, None);
let context = RevsetParseContext::new(
&aliases_map,
String::new(),
now.into(),
extensions,
None,
&fileset_aliases_map,
);
let expression = parse(&mut RevsetDiagnostics::new(), symbol, &context).unwrap();
assert_matches!(*expression, RevsetExpression::CommitRef(_));
let symbol_resolver = DefaultSymbolResolver::new(repo, extensions.symbol_resolvers());
Expand Down Expand Up @@ -206,13 +214,15 @@ fn test_resolve_symbol_commit_id() {
&([] as [&Box<dyn SymbolResolverExtension>; 0]),
);
let aliases_map = RevsetAliasesMap::default();
let fileset_aliases_map = FilesetAliasesMap::default();
let extensions = RevsetExtensions::default();
let context = RevsetParseContext::new(
&aliases_map,
settings.user_email(),
chrono::Utc::now().fixed_offset().into(),
&extensions,
None,
&fileset_aliases_map,
);
assert_matches!(
parse(&mut RevsetDiagnostics::new(), "present(04)", &context).unwrap()
Expand Down Expand Up @@ -921,13 +931,15 @@ fn try_resolve_commit_ids(
) -> Result<Vec<CommitId>, RevsetResolutionError> {
let settings = testutils::user_settings();
let aliases_map = RevsetAliasesMap::default();
let fileset_aliases_map = FilesetAliasesMap::default();
let revset_extensions = RevsetExtensions::default();
let context = RevsetParseContext::new(
&aliases_map,
settings.user_email(),
chrono::Utc::now().fixed_offset().into(),
&revset_extensions,
None,
&fileset_aliases_map,
);
let expression = parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap();
let symbol_resolver = DefaultSymbolResolver::new(repo, revset_extensions.symbol_resolvers());
Expand Down Expand Up @@ -956,13 +968,15 @@ fn resolve_commit_ids_in_workspace(
workspace_id: workspace.workspace_id(),
};
let aliases_map = RevsetAliasesMap::default();
let fileset_aliases_map = FilesetAliasesMap::default();
let extensions = RevsetExtensions::default();
let context = RevsetParseContext::new(
&aliases_map,
settings.user_email(),
chrono::Utc::now().fixed_offset().into(),
&extensions,
Some(workspace_ctx),
&fileset_aliases_map,
);
let expression = parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap();
let symbol_resolver =
Expand Down

0 comments on commit 6e0f91d

Please sign in to comment.