ok/jj
1
0
Fork 0
forked from mirrors/jj

revset: pass all context arguments to parse() via an object

`revset::parse()` already has a `RevsetWorkspaceContext` argument, so
I think it makes sense to put that and the other context arguments
into a larger `RevsetParseContext` object.
This commit is contained in:
Martin von Zweigbergk 2023-08-18 19:26:38 -07:00 committed by Martin von Zweigbergk
parent 5f3df4aaea
commit c43a3067eb
5 changed files with 60 additions and 70 deletions

View file

@ -48,8 +48,8 @@ use jj_lib::repo::{
use jj_lib::repo_path::{FsPathParseError, RepoPath};
use jj_lib::revset::{
DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression,
RevsetIteratorExt, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError,
RevsetWorkspaceContext,
RevsetIteratorExt, RevsetParseContext, RevsetParseError, RevsetParseErrorKind,
RevsetResolutionError, RevsetWorkspaceContext,
};
use jj_lib::settings::{ConfigResultExt as _, UserSettings};
use jj_lib::transaction::Transaction;
@ -1051,12 +1051,7 @@ impl WorkspaceCommandHelper {
revision_str: &str,
ui: Option<&mut Ui>,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let expression = revset::parse(
revision_str,
&self.revset_aliases_map,
&self.settings.user_email(),
Some(&self.revset_context()),
)?;
let expression = revset::parse(revision_str, &self.revset_parse_context())?;
if let Some(ui) = ui {
fn has_legacy_rule(expression: &Rc<RevsetExpression>) -> bool {
match expression.as_ref() {
@ -1125,15 +1120,16 @@ impl WorkspaceCommandHelper {
Ok(revset_expression.evaluate(self.repo().as_ref())?)
}
pub(crate) fn revset_aliases_map(&self) -> &RevsetAliasesMap {
&self.revset_aliases_map
}
pub(crate) fn revset_context(&self) -> RevsetWorkspaceContext {
RevsetWorkspaceContext {
pub(crate) fn revset_parse_context(&self) -> RevsetParseContext {
let workspace_context = RevsetWorkspaceContext {
cwd: &self.cwd,
workspace_id: self.workspace_id(),
workspace_root: self.workspace.workspace_root(),
};
RevsetParseContext {
aliases_map: &self.revset_aliases_map,
user_email: self.settings.user_email(),
workspace: Some(workspace_context),
}
}

View file

@ -199,17 +199,11 @@ fn cmd_debug_revset(
command: &CommandHelper,
args: &DebugRevsetArgs,
) -> Result<(), CommandError> {
let user_email = command.settings().user_email();
let workspace_command = command.workspace_helper(ui)?;
let workspace_ctx = workspace_command.revset_context();
let workspace_ctx = workspace_command.revset_parse_context();
let repo = workspace_command.repo().as_ref();
let expression = revset::parse(
&args.revision,
workspace_command.revset_aliases_map(),
&user_email,
Some(&workspace_ctx),
)?;
let expression = revset::parse(&args.revision, &workspace_ctx)?;
writeln!(ui, "-- Parsed:")?;
writeln!(ui, "{expression:#?}")?;
writeln!(ui)?;

View file

@ -40,9 +40,7 @@ use jj_lib::merge::Merge;
use jj_lib::op_store::WorkspaceId;
use jj_lib::repo::{ReadonlyRepo, Repo};
use jj_lib::repo_path::RepoPath;
use jj_lib::revset::{
RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt,
};
use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt};
use jj_lib::revset_graph::{
ReverseRevsetGraphIterator, RevsetGraphEdgeType, TopoGroupedRevsetGraphIterator,
};
@ -1724,13 +1722,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
the working copy commit, pass -r '@' instead."
)?;
} else if revset.is_empty()
&& revset::parse(
only_path,
&RevsetAliasesMap::new(),
&command.settings().user_email(),
Some(&workspace_command.revset_context()),
)
.is_ok()
&& revset::parse(only_path, &workspace_command.revset_parse_context()).is_ok()
{
writeln!(
ui.warning(),
@ -2521,9 +2513,7 @@ from the source will be moved into the parent.
if matches.value_source("revision").unwrap() == ValueSource::DefaultValue
&& revset::parse(
only_path,
&RevsetAliasesMap::new(),
&command.settings().user_email(),
Some(&tx.base_workspace_helper().revset_context()),
&tx.base_workspace_helper().revset_parse_context(),
)
.is_ok()
{

View file

@ -732,7 +732,7 @@ struct ParseState<'a> {
aliases_expanding: &'a [RevsetAliasId<'a>],
locals: &'a HashMap<&'a str, Rc<RevsetExpression>>,
user_email: &'a str,
workspace_ctx: Option<&'a RevsetWorkspaceContext<'a>>,
workspace_ctx: &'a Option<RevsetWorkspaceContext<'a>>,
}
impl ParseState<'_> {
@ -1387,16 +1387,14 @@ fn parse_function_argument_as_literal<T: FromStr>(
pub fn parse(
revset_str: &str,
aliases_map: &RevsetAliasesMap,
user_email: &str,
workspace_ctx: Option<&RevsetWorkspaceContext>,
context: &RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let state = ParseState {
aliases_map,
aliases_map: context.aliases_map,
aliases_expanding: &[],
locals: &HashMap::new(),
user_email,
workspace_ctx,
user_email: &context.user_email,
workspace_ctx: &context.workspace,
};
parse_program(revset_str, state)
}
@ -2390,6 +2388,14 @@ impl Iterator for ReverseRevsetIterator {
}
}
/// Information needed to parse revset expression.
#[derive(Clone, Debug)]
pub struct RevsetParseContext<'a> {
pub aliases_map: &'a RevsetAliasesMap,
pub user_email: String,
pub workspace: Option<RevsetWorkspaceContext<'a>>,
}
/// Workspace information needed to parse revset expression.
#[derive(Clone, Debug)]
pub struct RevsetWorkspaceContext<'a> {
@ -2421,8 +2427,13 @@ mod tests {
for (decl, defn) in aliases {
aliases_map.insert(decl, defn).unwrap();
}
let context = RevsetParseContext {
aliases_map: &aliases_map,
user_email: "test.user@example.com".to_string(),
workspace: None,
};
// Map error to comparable object
super::parse(revset_str, &aliases_map, "test.user@example.com", None).map_err(|e| e.kind)
super::parse(revset_str, &context).map_err(|e| e.kind)
}
fn parse_with_aliases_and_workspace(
@ -2440,14 +2451,13 @@ mod tests {
for (decl, defn) in aliases {
aliases_map.insert(decl, defn).unwrap();
}
let context = RevsetParseContext {
aliases_map: &aliases_map,
user_email: "test.user@example.com".to_string(),
workspace: Some(workspace_ctx),
};
// Map error to comparable object
super::parse(
revset_str,
&aliases_map,
"test.user@example.com",
Some(&workspace_ctx),
)
.map_err(|e| e.kind)
super::parse(revset_str, &context).map_err(|e| e.kind)
}
#[test]

View file

@ -31,7 +31,8 @@ use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPath;
use jj_lib::revset::{
optimize, parse, DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetExpression,
RevsetFilterPredicate, RevsetResolutionError, RevsetWorkspaceContext, SymbolResolver as _,
RevsetFilterPredicate, RevsetParseContext, RevsetResolutionError, RevsetWorkspaceContext,
SymbolResolver as _,
};
use jj_lib::revset_graph::{ReverseRevsetGraphIterator, RevsetGraphEdge};
use jj_lib::settings::GitSettings;
@ -172,8 +173,13 @@ fn test_resolve_symbol_commit_id() {
// Test present() suppresses only NoSuchRevision error
assert_eq!(resolve_commit_ids(repo.as_ref(), "present(foo)"), []);
let symbol_resolver = DefaultSymbolResolver::new(repo.as_ref());
let context = RevsetParseContext {
aliases_map: &RevsetAliasesMap::new(),
user_email: settings.user_email(),
workspace: None,
};
assert_matches!(
optimize(parse("present(04)", &RevsetAliasesMap::new(), &settings.user_email(), None).unwrap()).resolve_user_expression(repo.as_ref(), &symbol_resolver),
optimize(parse("present(04)", &context).unwrap()).resolve_user_expression(repo.as_ref(), &symbol_resolver),
Err(RevsetResolutionError::AmbiguousCommitIdPrefix(s)) if s == "04"
);
assert_eq!(
@ -734,15 +740,12 @@ fn test_resolve_symbol_git_refs() {
fn resolve_commit_ids(repo: &dyn Repo, revset_str: &str) -> Vec<CommitId> {
let settings = testutils::user_settings();
let expression = optimize(
parse(
revset_str,
&RevsetAliasesMap::new(),
&settings.user_email(),
None,
)
.unwrap(),
);
let context = RevsetParseContext {
aliases_map: &RevsetAliasesMap::new(),
user_email: settings.user_email(),
workspace: None,
};
let expression = optimize(parse(revset_str, &context).unwrap());
let symbol_resolver = DefaultSymbolResolver::new(repo);
let expression = expression
.resolve_user_expression(repo, &symbol_resolver)
@ -762,15 +765,12 @@ fn resolve_commit_ids_in_workspace(
workspace_id: workspace.workspace_id(),
workspace_root: workspace.workspace_root(),
};
let expression = optimize(
parse(
revset_str,
&RevsetAliasesMap::new(),
&settings.user_email(),
Some(&workspace_ctx),
)
.unwrap(),
);
let context = RevsetParseContext {
aliases_map: &RevsetAliasesMap::new(),
user_email: settings.user_email(),
workspace: Some(workspace_ctx),
};
let expression = optimize(parse(revset_str, &context).unwrap());
let symbol_resolver = DefaultSymbolResolver::new(repo);
let expression = expression
.resolve_user_expression(repo, &symbol_resolver)