From 7632466cc07079f0c015eefcf70796fe12d96b42 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 25 Nov 2022 12:53:01 +0900 Subject: [PATCH] revset: add table of symbol aliases and pass around parse functions The CLI will load aliases from config, insert them one by one, and warn if declaration part is invalid. That's why RevsetAliasesMap is a public struct and needs to be instantiated by the caller. --- lib/src/revset.pest | 4 ++ lib/src/revset.rs | 89 ++++++++++++++++++++++++++++++++++++++-- lib/tests/test_revset.rs | 10 +++-- src/cli_util.rs | 6 ++- src/commands.rs | 6 ++- 5 files changed, 104 insertions(+), 11 deletions(-) diff --git a/lib/src/revset.pest b/lib/src/revset.pest index f1af7d09b..de2ad6aac 100644 --- a/lib/src/revset.pest +++ b/lib/src/revset.pest @@ -68,3 +68,7 @@ expression = { } program = _{ SOI ~ expression ~ EOI } + +alias_declaration = _{ + SOI ~ identifier ~ EOI +} diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 323ea4139..8b749ec85 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -14,7 +14,7 @@ use std::borrow::Borrow; use std::cmp::{Ordering, Reverse}; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::iter::Peekable; use std::ops::Range; use std::path::Path; @@ -474,8 +474,76 @@ impl RevsetExpression { } } +#[derive(Clone, Debug, Default)] +pub struct RevsetAliasesMap { + symbol_aliases: HashMap, +} + +impl RevsetAliasesMap { + pub fn new() -> Self { + Self::default() + } + + /// Adds new substitution rule `decl = defn`. + /// + /// Returns error if `decl` is invalid. The `defn` part isn't checked. A bad + /// `defn` will be reported when the alias is substituted. + pub fn insert( + &mut self, + decl: impl AsRef, + defn: impl Into, + ) -> Result<(), RevsetParseError> { + match RevsetAliasDeclaration::parse(decl.as_ref())? { + RevsetAliasDeclaration::Symbol(name) => { + self.symbol_aliases.insert(name, defn.into()); + } + } + Ok(()) + } + + fn get_symbol<'a>(&'a self, name: &str) -> Option<(RevsetAliasId<'a>, &'a str)> { + self.symbol_aliases + .get_key_value(name) + .map(|(name, defn)| (RevsetAliasId::Symbol(name), defn.as_ref())) + } +} + +/// Parsed declaration part of alias rule. +#[derive(Clone, Debug)] +enum RevsetAliasDeclaration { + Symbol(String), + // TODO: Function(String, Vec) +} + +impl RevsetAliasDeclaration { + fn parse(source: &str) -> Result { + let mut pairs = RevsetParser::parse(Rule::alias_declaration, source)?; + let first = pairs.next().unwrap(); + match first.as_rule() { + Rule::identifier => Ok(RevsetAliasDeclaration::Symbol(first.as_str().to_owned())), + r => panic!("unxpected alias declaration rule {r:?}"), + } + } +} + +/// Borrowed reference to identify alias expression. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RevsetAliasId<'a> { + Symbol(&'a str), + // TODO: Function(&'a str) +} + +impl fmt::Display for RevsetAliasId<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + RevsetAliasId::Symbol(name) => write!(f, "{name}"), + } + } +} + #[derive(Clone, Copy, Debug)] struct ParseState<'a> { + aliases_map: &'a RevsetAliasesMap, workspace_ctx: Option<&'a RevsetWorkspaceContext<'a>>, } @@ -791,11 +859,15 @@ fn parse_function_argument_to_string( pub fn parse( revset_str: &str, + aliases_map: &RevsetAliasesMap, workspace_ctx: Option<&RevsetWorkspaceContext>, ) -> Result, RevsetParseError> { let mut pairs = RevsetParser::parse(Rule::program, revset_str)?; let first = pairs.next().unwrap(); - let state = ParseState { workspace_ctx }; + let state = ParseState { + aliases_map, + workspace_ctx, + }; parse_expression_rule(first.into_inner(), state) } @@ -1624,6 +1696,17 @@ mod tests { use super::*; fn parse(revset_str: &str) -> Result, RevsetParseErrorKind> { + parse_with_aliases(revset_str, [] as [(&str, &str); 0]) + } + + fn parse_with_aliases( + revset_str: &str, + aliases: impl IntoIterator, impl Into)>, + ) -> Result, RevsetParseErrorKind> { + let mut aliases_map = RevsetAliasesMap::new(); + for (decl, defn) in aliases { + aliases_map.insert(decl, defn).unwrap(); + } // Set up pseudo context to resolve file(path) let workspace_ctx = RevsetWorkspaceContext { cwd: Path::new("/"), @@ -1631,7 +1714,7 @@ mod tests { workspace_root: Path::new("/"), }; // Map error to comparable object - super::parse(revset_str, Some(&workspace_ctx)).map_err(|e| e.kind) + super::parse(revset_str, &aliases_map, Some(&workspace_ctx)).map_err(|e| e.kind) } #[test] diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index bd8181c50..a9ffda7e4 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -22,7 +22,8 @@ use jujutsu_lib::op_store::{RefTarget, WorkspaceId}; use jujutsu_lib::repo::RepoRef; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::revset::{ - self, optimize, parse, resolve_symbol, RevsetError, RevsetExpression, RevsetWorkspaceContext, + self, optimize, parse, resolve_symbol, RevsetAliasesMap, RevsetError, RevsetExpression, + RevsetWorkspaceContext, }; use jujutsu_lib::workspace::Workspace; use test_case::test_case; @@ -135,7 +136,7 @@ fn test_resolve_symbol_commit_id() { // Test present() suppresses only NoSuchRevision error assert_eq!(resolve_commit_ids(repo_ref, "present(foo)"), []); assert_eq!( - optimize(parse("present(04)", None).unwrap()) + optimize(parse("present(04)", &RevsetAliasesMap::new(), None).unwrap()) .evaluate(repo_ref, None) .map(|_| ()), Err(RevsetError::AmbiguousCommitIdPrefix("04".to_string())) @@ -443,7 +444,7 @@ fn test_resolve_symbol_git_refs() { } fn resolve_commit_ids(repo: RepoRef, revset_str: &str) -> Vec { - let expression = optimize(parse(revset_str, None).unwrap()); + let expression = optimize(parse(revset_str, &RevsetAliasesMap::new(), None).unwrap()); expression .evaluate(repo, None) .unwrap() @@ -463,7 +464,8 @@ fn resolve_commit_ids_in_workspace( workspace_id: workspace.workspace_id(), workspace_root: workspace.workspace_root(), }; - let expression = optimize(parse(revset_str, Some(&workspace_ctx)).unwrap()); + let expression = + optimize(parse(revset_str, &RevsetAliasesMap::new(), Some(&workspace_ctx)).unwrap()); expression .evaluate(repo, Some(&workspace_ctx)) .unwrap() diff --git a/src/cli_util.rs b/src/cli_util.rs index 94b59f0a1..abe3d3a04 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -36,7 +36,8 @@ use jujutsu_lib::operation::Operation; use jujutsu_lib::repo::{BackendFactories, MutableRepo, ReadonlyRepo, RepoRef, RewriteRootCommit}; use jujutsu_lib::repo_path::{FsPathParseError, RepoPath}; use jujutsu_lib::revset::{ - Revset, RevsetError, RevsetExpression, RevsetParseError, RevsetWorkspaceContext, + Revset, RevsetAliasesMap, RevsetError, RevsetExpression, RevsetParseError, + RevsetWorkspaceContext, }; use jujutsu_lib::settings::UserSettings; use jujutsu_lib::transaction::Transaction; @@ -606,7 +607,8 @@ impl WorkspaceCommandHelper { &self, revision_str: &str, ) -> Result, RevsetParseError> { - let expression = revset::parse(revision_str, Some(&self.revset_context()))?; + let aliases_map = RevsetAliasesMap::new(); // TODO: load from settings + let expression = revset::parse(revision_str, &aliases_map, Some(&self.revset_context()))?; Ok(revset::optimize(expression)) } diff --git a/src/commands.rs b/src/commands.rs index 17e5ab7f7..4b1773ea4 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -40,7 +40,7 @@ use jujutsu_lib::operation::Operation; use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate}; use jujutsu_lib::repo::{ReadonlyRepo, RepoRef}; use jujutsu_lib::repo_path::RepoPath; -use jujutsu_lib::revset::RevsetExpression; +use jujutsu_lib::revset::{RevsetAliasesMap, RevsetExpression}; use jujutsu_lib::revset_graph_iterator::{RevsetGraphEdge, RevsetGraphEdgeType}; use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser}; use jujutsu_lib::settings::UserSettings; @@ -2164,7 +2164,9 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C often not useful because all non-empty commits touch '.'. If you meant to show \ the working copy commit, pass -r '@' instead.\n" ))?; - } else if revset.is_empty() && revset::parse(only_path, None).is_ok() { + } else if revset.is_empty() + && revset::parse(only_path, &RevsetAliasesMap::new(), None).is_ok() + { ui.write_warn(&format!( "warning: The argument {only_path:?} is being interpreted as a path. To specify a \ revset, pass -r {only_path:?} instead.\n"