mirror of
https://github.com/martinvonz/jj.git
synced 2025-01-20 11:25:34 +00:00
id_prefix: remove repo
field from IdPrefixContext
By passing the repo as argument to the methods instead, we can remove the `repo` field and the associated lifetime. Thanks to Yuya for the suggestion.
This commit is contained in:
parent
eab5218fe5
commit
916b00c33e
5 changed files with 124 additions and 70 deletions
|
@ -66,19 +66,12 @@ impl DisambiguationData {
|
|||
}
|
||||
}
|
||||
|
||||
pub struct IdPrefixContext<'repo> {
|
||||
repo: &'repo dyn Repo,
|
||||
#[derive(Default)]
|
||||
pub struct IdPrefixContext {
|
||||
disambiguation: Option<DisambiguationData>,
|
||||
}
|
||||
|
||||
impl IdPrefixContext<'_> {
|
||||
pub fn new(repo: &dyn Repo) -> IdPrefixContext {
|
||||
IdPrefixContext {
|
||||
repo,
|
||||
disambiguation: None,
|
||||
}
|
||||
}
|
||||
|
||||
impl IdPrefixContext {
|
||||
pub fn disambiguate_within(
|
||||
mut self,
|
||||
expression: Rc<RevsetExpression>,
|
||||
|
@ -92,59 +85,65 @@ impl IdPrefixContext<'_> {
|
|||
self
|
||||
}
|
||||
|
||||
fn disambiguation_indexes(&self) -> Option<&Indexes> {
|
||||
fn disambiguation_indexes(&self, repo: &dyn Repo) -> Option<&Indexes> {
|
||||
// TODO: propagate errors instead of treating them as if no revset was specified
|
||||
self.disambiguation
|
||||
.as_ref()
|
||||
.and_then(|disambiguation| disambiguation.indexes(self.repo).ok())
|
||||
.and_then(|disambiguation| disambiguation.indexes(repo).ok())
|
||||
}
|
||||
|
||||
/// Resolve an unambiguous commit ID prefix.
|
||||
pub fn resolve_commit_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId> {
|
||||
if let Some(indexes) = self.disambiguation_indexes() {
|
||||
pub fn resolve_commit_prefix(
|
||||
&self,
|
||||
repo: &dyn Repo,
|
||||
prefix: &HexPrefix,
|
||||
) -> PrefixResolution<CommitId> {
|
||||
if let Some(indexes) = self.disambiguation_indexes(repo) {
|
||||
let resolution = indexes.commit_index.resolve_prefix(prefix);
|
||||
if let PrefixResolution::SingleMatch(mut ids) = resolution {
|
||||
assert_eq!(ids.len(), 1);
|
||||
return PrefixResolution::SingleMatch(ids.pop().unwrap());
|
||||
}
|
||||
}
|
||||
self.repo.index().resolve_prefix(prefix)
|
||||
repo.index().resolve_prefix(prefix)
|
||||
}
|
||||
|
||||
/// Returns the shortest length of a prefix of `commit_id` that
|
||||
/// can still be resolved by `resolve_commit_prefix()`.
|
||||
pub fn shortest_commit_prefix_len(&self, commit_id: &CommitId) -> usize {
|
||||
if let Some(indexes) = self.disambiguation_indexes() {
|
||||
pub fn shortest_commit_prefix_len(&self, repo: &dyn Repo, commit_id: &CommitId) -> usize {
|
||||
if let Some(indexes) = self.disambiguation_indexes(repo) {
|
||||
// TODO: Avoid the double lookup here (has_key() + shortest_unique_prefix_len())
|
||||
if indexes.commit_index.has_key(commit_id) {
|
||||
return indexes.commit_index.shortest_unique_prefix_len(commit_id);
|
||||
}
|
||||
}
|
||||
self.repo
|
||||
.index()
|
||||
.shortest_unique_commit_id_prefix_len(commit_id)
|
||||
repo.index().shortest_unique_commit_id_prefix_len(commit_id)
|
||||
}
|
||||
|
||||
/// Resolve an unambiguous change ID prefix to the commit IDs in the revset.
|
||||
pub fn resolve_change_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
|
||||
if let Some(indexes) = self.disambiguation_indexes() {
|
||||
pub fn resolve_change_prefix(
|
||||
&self,
|
||||
repo: &dyn Repo,
|
||||
prefix: &HexPrefix,
|
||||
) -> PrefixResolution<Vec<CommitId>> {
|
||||
if let Some(indexes) = self.disambiguation_indexes(repo) {
|
||||
let resolution = indexes.change_index.resolve_prefix(prefix);
|
||||
if let PrefixResolution::SingleMatch(ids) = resolution {
|
||||
return PrefixResolution::SingleMatch(ids);
|
||||
}
|
||||
}
|
||||
self.repo.resolve_change_id_prefix(prefix)
|
||||
repo.resolve_change_id_prefix(prefix)
|
||||
}
|
||||
|
||||
/// Returns the shortest length of a prefix of `change_id` that
|
||||
/// can still be resolved by `resolve_change_prefix()`.
|
||||
pub fn shortest_change_prefix_len(&self, change_id: &ChangeId) -> usize {
|
||||
if let Some(indexes) = self.disambiguation_indexes() {
|
||||
pub fn shortest_change_prefix_len(&self, repo: &dyn Repo, change_id: &ChangeId) -> usize {
|
||||
if let Some(indexes) = self.disambiguation_indexes(repo) {
|
||||
if indexes.change_index.has_key(change_id) {
|
||||
return indexes.change_index.shortest_unique_prefix_len(change_id);
|
||||
}
|
||||
}
|
||||
self.repo.shortest_unique_change_id_prefix_len(change_id)
|
||||
repo.shortest_unique_change_id_prefix_len(change_id)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -1647,7 +1647,7 @@ impl SymbolResolver for FailingSymbolResolver {
|
|||
}
|
||||
}
|
||||
|
||||
pub type PrefixResolver<'a, T> = Box<dyn Fn(&HexPrefix) -> PrefixResolution<T> + 'a>;
|
||||
pub type PrefixResolver<'a, T> = Box<dyn Fn(&dyn Repo, &HexPrefix) -> PrefixResolution<T> + 'a>;
|
||||
|
||||
/// Resolves the "root" and "@" symbols, branches, remote branches, tags, git
|
||||
/// refs, and full and abbreviated commit and change ids.
|
||||
|
@ -1728,7 +1728,7 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
|
|||
// Try to resolve as a commit id.
|
||||
if let Some(prefix) = HexPrefix::new(symbol) {
|
||||
let prefix_resolution = if let Some(commit_id_resolver) = &self.commit_id_resolver {
|
||||
commit_id_resolver(&prefix)
|
||||
commit_id_resolver(self.repo, &prefix)
|
||||
} else {
|
||||
self.repo.index().resolve_prefix(&prefix)
|
||||
};
|
||||
|
@ -1748,7 +1748,7 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
|
|||
// Try to resolve as a change id.
|
||||
if let Some(prefix) = to_forward_hex(symbol).as_deref().and_then(HexPrefix::new) {
|
||||
let prefix_resolution = if let Some(change_id_resolver) = &self.change_id_resolver {
|
||||
change_id_resolver(&prefix)
|
||||
change_id_resolver(self.repo, &prefix)
|
||||
} else {
|
||||
self.repo.resolve_change_id_prefix(&prefix)
|
||||
};
|
||||
|
|
|
@ -130,25 +130,55 @@ fn test_id_prefix() {
|
|||
|
||||
// Without a disambiguation revset
|
||||
// ---------------------------------------------------------------------------------------------
|
||||
let c = IdPrefixContext::new(repo.as_ref());
|
||||
assert_eq!(c.shortest_commit_prefix_len(commits[2].id()), 2);
|
||||
assert_eq!(c.shortest_commit_prefix_len(commits[5].id()), 1);
|
||||
assert_eq!(c.resolve_commit_prefix(&prefix("2")), AmbiguousMatch);
|
||||
let c = IdPrefixContext::default();
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(&prefix("2a")),
|
||||
c.shortest_commit_prefix_len(repo.as_ref(), commits[2].id()),
|
||||
2
|
||||
);
|
||||
assert_eq!(
|
||||
c.shortest_commit_prefix_len(repo.as_ref(), commits[5].id()),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("2")),
|
||||
AmbiguousMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("2a")),
|
||||
SingleMatch(commits[2].id().clone())
|
||||
);
|
||||
assert_eq!(c.resolve_commit_prefix(&prefix("20")), NoMatch);
|
||||
assert_eq!(c.resolve_commit_prefix(&prefix("2a0")), NoMatch);
|
||||
assert_eq!(c.shortest_change_prefix_len(commits[0].change_id()), 2);
|
||||
assert_eq!(c.shortest_change_prefix_len(commits[6].change_id()), 1);
|
||||
assert_eq!(c.resolve_change_prefix(&prefix("7")), AmbiguousMatch);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(&prefix("78")),
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("20")),
|
||||
NoMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("2a0")),
|
||||
NoMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.shortest_change_prefix_len(repo.as_ref(), commits[0].change_id()),
|
||||
2
|
||||
);
|
||||
assert_eq!(
|
||||
c.shortest_change_prefix_len(repo.as_ref(), commits[6].change_id()),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("7")),
|
||||
AmbiguousMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("78")),
|
||||
SingleMatch(vec![commits[0].id().clone()])
|
||||
);
|
||||
assert_eq!(c.resolve_change_prefix(&prefix("70")), NoMatch);
|
||||
assert_eq!(c.resolve_change_prefix(&prefix("780")), NoMatch);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("70")),
|
||||
NoMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("780")),
|
||||
NoMatch
|
||||
);
|
||||
|
||||
// Disambiguate within a revset
|
||||
// ---------------------------------------------------------------------------------------------
|
||||
|
@ -156,20 +186,26 @@ fn test_id_prefix() {
|
|||
RevsetExpression::commits(vec![commits[0].id().clone(), commits[2].id().clone()]);
|
||||
let c = c.disambiguate_within(expression, None);
|
||||
// The prefix is now shorter
|
||||
assert_eq!(c.shortest_commit_prefix_len(commits[2].id()), 1);
|
||||
assert_eq!(
|
||||
c.shortest_commit_prefix_len(repo.as_ref(), commits[2].id()),
|
||||
1
|
||||
);
|
||||
// Shorter prefix within the set can be used
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(&prefix("2")),
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("2")),
|
||||
SingleMatch(commits[2].id().clone())
|
||||
);
|
||||
// Can still resolve commits outside the set
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(&prefix("21")),
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("21")),
|
||||
SingleMatch(commits[24].id().clone())
|
||||
);
|
||||
assert_eq!(c.shortest_change_prefix_len(commits[0].change_id()), 1);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(&prefix("7")),
|
||||
c.shortest_change_prefix_len(repo.as_ref(), commits[0].change_id()),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("7")),
|
||||
SingleMatch(vec![commits[0].id().clone()])
|
||||
);
|
||||
|
||||
|
@ -178,16 +214,28 @@ fn test_id_prefix() {
|
|||
// ---------------------------------------------------------------------------------------------
|
||||
let expression = RevsetExpression::commit(root_commit_id.clone());
|
||||
let c = c.disambiguate_within(expression, None);
|
||||
assert_eq!(c.shortest_commit_prefix_len(root_commit_id), 1);
|
||||
assert_eq!(c.resolve_commit_prefix(&prefix("")), AmbiguousMatch);
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(&prefix("0")),
|
||||
c.shortest_commit_prefix_len(repo.as_ref(), root_commit_id),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("")),
|
||||
AmbiguousMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_commit_prefix(repo.as_ref(), &prefix("0")),
|
||||
SingleMatch(root_commit_id.clone())
|
||||
);
|
||||
assert_eq!(c.shortest_change_prefix_len(root_change_id), 1);
|
||||
assert_eq!(c.resolve_change_prefix(&prefix("")), AmbiguousMatch);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(&prefix("0")),
|
||||
c.shortest_change_prefix_len(repo.as_ref(), root_change_id),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("")),
|
||||
AmbiguousMatch
|
||||
);
|
||||
assert_eq!(
|
||||
c.resolve_change_prefix(repo.as_ref(), &prefix("0")),
|
||||
SingleMatch(vec![root_commit_id.clone()])
|
||||
);
|
||||
|
||||
|
@ -196,6 +244,12 @@ fn test_id_prefix() {
|
|||
// TODO: Should be an error
|
||||
let expression = RevsetExpression::symbol("nonexistent".to_string());
|
||||
let context = c.disambiguate_within(expression, None);
|
||||
assert_eq!(context.shortest_commit_prefix_len(commits[2].id()), 2);
|
||||
assert_eq!(context.resolve_commit_prefix(&prefix("2")), AmbiguousMatch);
|
||||
assert_eq!(
|
||||
context.shortest_commit_prefix_len(repo.as_ref(), commits[2].id()),
|
||||
2
|
||||
);
|
||||
assert_eq!(
|
||||
context.resolve_commit_prefix(repo.as_ref(), &prefix("2")),
|
||||
AmbiguousMatch
|
||||
);
|
||||
}
|
||||
|
|
|
@ -548,7 +548,7 @@ impl CommandHelper {
|
|||
/// data is lazily loaded.
|
||||
struct ReadonlyUserRepo {
|
||||
repo: Arc<ReadonlyRepo>,
|
||||
id_prefix_context: OnceCell<IdPrefixContext<'static>>,
|
||||
id_prefix_context: OnceCell<IdPrefixContext>,
|
||||
}
|
||||
|
||||
impl ReadonlyUserRepo {
|
||||
|
@ -590,7 +590,7 @@ impl WorkspaceCommandHelper {
|
|||
// Parse commit_summary template early to report error before starting mutable
|
||||
// operation.
|
||||
// TODO: Parsed template can be cached if it doesn't capture repo
|
||||
let id_prefix_context = IdPrefixContext::new(repo.as_ref());
|
||||
let id_prefix_context = IdPrefixContext::default();
|
||||
parse_commit_summary_template(
|
||||
repo.as_ref(),
|
||||
workspace.workspace_id(),
|
||||
|
@ -925,18 +925,18 @@ impl WorkspaceCommandHelper {
|
|||
|
||||
pub(crate) fn revset_symbol_resolver(&self) -> DefaultSymbolResolver<'_> {
|
||||
let id_prefix_context = self.id_prefix_context();
|
||||
let commit_id_resolver: revset::PrefixResolver<'_, CommitId> =
|
||||
Box::new(|prefix| id_prefix_context.resolve_commit_prefix(prefix));
|
||||
let change_id_resolver: revset::PrefixResolver<'_, Vec<CommitId>> =
|
||||
Box::new(|prefix| id_prefix_context.resolve_change_prefix(prefix));
|
||||
let commit_id_resolver: revset::PrefixResolver<CommitId> =
|
||||
Box::new(|repo, prefix| id_prefix_context.resolve_commit_prefix(repo, prefix));
|
||||
let change_id_resolver: revset::PrefixResolver<Vec<CommitId>> =
|
||||
Box::new(|repo, prefix| id_prefix_context.resolve_change_prefix(repo, prefix));
|
||||
DefaultSymbolResolver::new(self.repo().as_ref(), Some(self.workspace_id()))
|
||||
.with_commit_id_resolver(commit_id_resolver)
|
||||
.with_change_id_resolver(change_id_resolver)
|
||||
}
|
||||
|
||||
pub fn id_prefix_context(&self) -> &IdPrefixContext<'_> {
|
||||
pub fn id_prefix_context(&self) -> &IdPrefixContext {
|
||||
self.user_repo.id_prefix_context.get_or_init(|| {
|
||||
let mut context: IdPrefixContext<'_> = IdPrefixContext::new(self.repo().as_ref());
|
||||
let mut context: IdPrefixContext = IdPrefixContext::default();
|
||||
let revset_string: String = self
|
||||
.settings
|
||||
.config()
|
||||
|
@ -947,7 +947,6 @@ impl WorkspaceCommandHelper {
|
|||
context = context
|
||||
.disambiguate_within(disambiguation_revset, Some(self.workspace_id().clone()));
|
||||
}
|
||||
let context: IdPrefixContext<'static> = unsafe { std::mem::transmute(context) };
|
||||
context
|
||||
})
|
||||
}
|
||||
|
@ -1295,7 +1294,7 @@ impl WorkspaceCommandTransaction<'_> {
|
|||
commit: &Commit,
|
||||
) -> std::io::Result<()> {
|
||||
// TODO: Use the disambiguation revset
|
||||
let id_prefix_context = IdPrefixContext::new(self.tx.repo());
|
||||
let id_prefix_context = IdPrefixContext::default();
|
||||
let template = parse_commit_summary_template(
|
||||
self.tx.repo(),
|
||||
self.helper.workspace_id(),
|
||||
|
@ -1734,7 +1733,7 @@ fn load_template_aliases(
|
|||
fn parse_commit_summary_template<'a>(
|
||||
repo: &'a dyn Repo,
|
||||
workspace_id: &WorkspaceId,
|
||||
id_prefix_context: &'a IdPrefixContext<'a>,
|
||||
id_prefix_context: &'a IdPrefixContext,
|
||||
aliases_map: &TemplateAliasesMap,
|
||||
settings: &UserSettings,
|
||||
) -> Result<Box<dyn Template<Commit> + 'a>, CommandError> {
|
||||
|
|
|
@ -40,7 +40,7 @@ use crate::text_util;
|
|||
struct CommitTemplateLanguage<'repo, 'b> {
|
||||
repo: &'repo dyn Repo,
|
||||
workspace_id: &'b WorkspaceId,
|
||||
id_prefix_context: &'repo IdPrefixContext<'repo>,
|
||||
id_prefix_context: &'repo IdPrefixContext,
|
||||
}
|
||||
|
||||
impl<'repo> TemplateLanguage<'repo> for CommitTemplateLanguage<'repo, '_> {
|
||||
|
@ -387,13 +387,14 @@ impl CommitOrChangeId {
|
|||
/// length of the shortest unique prefix
|
||||
pub fn shortest(
|
||||
&self,
|
||||
repo: &dyn Repo,
|
||||
id_prefix_context: &IdPrefixContext,
|
||||
total_len: usize,
|
||||
) -> ShortestIdPrefix {
|
||||
let mut hex = self.hex();
|
||||
let prefix_len = match self {
|
||||
CommitOrChangeId::Commit(id) => id_prefix_context.shortest_commit_prefix_len(id),
|
||||
CommitOrChangeId::Change(id) => id_prefix_context.shortest_change_prefix_len(id),
|
||||
CommitOrChangeId::Commit(id) => id_prefix_context.shortest_commit_prefix_len(repo, id),
|
||||
CommitOrChangeId::Change(id) => id_prefix_context.shortest_change_prefix_len(repo, id),
|
||||
};
|
||||
hex.truncate(max(prefix_len, total_len));
|
||||
let rest = hex.split_off(prefix_len);
|
||||
|
@ -434,6 +435,7 @@ fn build_commit_or_change_id_method<'repo>(
|
|||
(self_property, len_property),
|
||||
|(id, len)| {
|
||||
id.shortest(
|
||||
language.repo,
|
||||
id_prefix_context,
|
||||
len.and_then(|l| l.try_into().ok()).unwrap_or(0),
|
||||
)
|
||||
|
@ -515,7 +517,7 @@ fn build_shortest_id_prefix_method<'repo>(
|
|||
pub fn parse<'repo>(
|
||||
repo: &'repo dyn Repo,
|
||||
workspace_id: &WorkspaceId,
|
||||
id_prefix_context: &'repo IdPrefixContext<'repo>,
|
||||
id_prefix_context: &'repo IdPrefixContext,
|
||||
template_text: &str,
|
||||
aliases_map: &TemplateAliasesMap,
|
||||
) -> TemplateParseResult<Box<dyn Template<Commit> + 'repo>> {
|
||||
|
|
Loading…
Reference in a new issue