Skip to content

Commit

Permalink
cli_util: support multiple extensions consistently
Browse files Browse the repository at this point in the history
If we ever implement some sort of ABI for dynamic extension loading, we'll need these underlying APIs to support multiple extensions, so we might as well do that first.
  • Loading branch information
torquestomp committed Apr 11, 2024
1 parent 8fa256e commit 3341bff
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 67 deletions.
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"rust-analyzer.showUnlinkedFileNotification": false
}
4 changes: 2 additions & 2 deletions cli/examples/custom-backend/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ enum CustomCommand {
}

fn create_store_factories() -> StoreFactories {
let mut store_factories = StoreFactories::default();
let mut store_factories = StoreFactories::empty();
// Register the backend so it can be loaded when the repo is loaded. The name
// must match `Backend::name()`.
store_factories.add_backend(
Expand Down Expand Up @@ -73,7 +73,7 @@ fn run_custom_command(

fn main() -> std::process::ExitCode {
CliRunner::init()
.set_store_factories(create_store_factories())
.add_store_factories(create_store_factories())
.add_subcommand(run_custom_command)
.run()
}
Expand Down
2 changes: 1 addition & 1 deletion cli/examples/custom-commit-templater/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,6 @@ impl CommitTemplateLanguageExtension for HexCounter {

fn main() -> std::process::ExitCode {
CliRunner::init()
.set_commit_template_extension(Box::new(HexCounter))
.add_commit_template_extension(Box::new(HexCounter))
.run()
}
2 changes: 1 addition & 1 deletion cli/examples/custom-operation-templater/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,6 @@ impl OperationTemplateLanguageExtension for HexCounter {

fn main() -> std::process::ExitCode {
CliRunner::init()
.set_operation_template_extension(Box::new(HexCounter))
.add_operation_template_extension(Box::new(HexCounter))
.run()
}
6 changes: 3 additions & 3 deletions cli/examples/custom-working-copy/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use jj_lib::working_copy::{
CheckoutError, CheckoutStats, LockedWorkingCopy, ResetError, SnapshotError, SnapshotOptions,
WorkingCopy, WorkingCopyFactory, WorkingCopyStateError,
};
use jj_lib::workspace::{default_working_copy_factories, Workspace, WorkspaceInitError};
use jj_lib::workspace::{WorkingCopyFactories, Workspace, WorkspaceInitError};

#[derive(clap::Parser, Clone, Debug)]
enum CustomCommand {
Expand Down Expand Up @@ -74,13 +74,13 @@ fn run_custom_command(
}

fn main() -> std::process::ExitCode {
let mut working_copy_factories = default_working_copy_factories();
let mut working_copy_factories = WorkingCopyFactories::new();
working_copy_factories.insert(
ConflictsWorkingCopy::name().to_owned(),
Box::new(ConflictsWorkingCopyFactory {}),
);
CliRunner::init()
.set_working_copy_factories(working_copy_factories)
.add_working_copy_factories(working_copy_factories)
.add_subcommand(run_custom_command)
.run()
}
Expand Down
92 changes: 47 additions & 45 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use core::fmt;
use std::borrow::Cow;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::collections::{BTreeMap, HashSet};
use std::env::{self, ArgsOs, VarError};
use std::ffi::OsString;
use std::fmt::Debug;
Expand Down Expand Up @@ -49,8 +49,8 @@ use jj_lib::op_store::{OpStoreError, OperationId, WorkspaceId};
use jj_lib::op_walk::OpsetEvaluationError;
use jj_lib::operation::Operation;
use jj_lib::repo::{
CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, Repo, RepoLoader,
StoreFactories, StoreLoadError,
merge_factories_map, CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, Repo,
RepoLoader, StoreFactories, StoreLoadError,
};
use jj_lib::repo_path::{FsPathParseError, RepoPath, RepoPathBuf};
use jj_lib::revset::{
Expand All @@ -67,7 +67,8 @@ use jj_lib::working_copy::{
CheckoutStats, LockedWorkingCopy, SnapshotOptions, WorkingCopy, WorkingCopyFactory,
};
use jj_lib::workspace::{
default_working_copy_factories, LockedWorkspace, Workspace, WorkspaceLoadError, WorkspaceLoader,
default_working_copy_factories, LockedWorkspace, WorkingCopyFactories, Workspace,
WorkspaceLoadError, WorkspaceLoader,
};
use jj_lib::{dag_walk, file_util, fileset, git, op_heads_store, op_walk, revset};
use once_cell::unsync::OnceCell;
Expand Down Expand Up @@ -192,11 +193,11 @@ pub struct CommandHelper {
global_args: GlobalArgs,
settings: UserSettings,
layered_configs: LayeredConfigs,
commit_template_extension: Option<Arc<dyn CommitTemplateLanguageExtension>>,
operation_template_extension: Option<Arc<dyn OperationTemplateLanguageExtension>>,
commit_template_extensions: Vec<Arc<dyn CommitTemplateLanguageExtension>>,
operation_template_extensions: Vec<Arc<dyn OperationTemplateLanguageExtension>>,
maybe_workspace_loader: Result<WorkspaceLoader, CommandError>,
store_factories: StoreFactories,
working_copy_factories: HashMap<String, Box<dyn WorkingCopyFactory>>,
working_copy_factories: WorkingCopyFactories,
}

impl CommandHelper {
Expand Down Expand Up @@ -264,8 +265,10 @@ impl CommandHelper {
)?)
}

pub fn operation_template_extension(&self) -> Option<&dyn OperationTemplateLanguageExtension> {
self.operation_template_extension.as_deref()
pub fn operation_template_extensions(
&self,
) -> &Vec<Arc<dyn OperationTemplateLanguageExtension>> {
&self.operation_template_extensions
}

pub fn workspace_loader(&self) -> Result<&WorkspaceLoader, CommandError> {
Expand Down Expand Up @@ -403,7 +406,7 @@ pub struct WorkspaceCommandHelper {
user_repo: ReadonlyUserRepo,
// TODO: Parsed template can be cached if it doesn't capture 'repo lifetime
commit_summary_template_text: String,
commit_template_extension: Option<Arc<dyn CommitTemplateLanguageExtension>>,
commit_template_extensions: Vec<Arc<dyn CommitTemplateLanguageExtension>>,
revset_aliases_map: RevsetAliasesMap,
template_aliases_map: TemplateAliasesMap,
may_update_working_copy: bool,
Expand Down Expand Up @@ -434,7 +437,7 @@ impl WorkspaceCommandHelper {
workspace,
user_repo: ReadonlyUserRepo::new(repo),
commit_summary_template_text,
commit_template_extension: command.commit_template_extension.clone(),
commit_template_extensions: command.commit_template_extensions.clone(),
revset_aliases_map,
template_aliases_map,
may_update_working_copy,
Expand Down Expand Up @@ -956,7 +959,7 @@ impl WorkspaceCommandHelper {
self.workspace_id(),
self.revset_parse_context(),
self.id_prefix_context()?,
self.commit_template_extension.as_deref(),
&self.commit_template_extensions,
))
}

Expand Down Expand Up @@ -1436,7 +1439,7 @@ impl WorkspaceCommandTransaction<'_> {
self.helper.workspace_id(),
self.helper.revset_parse_context(),
&id_prefix_context,
self.helper.commit_template_extension.as_deref(),
&self.helper.commit_template_extensions,
);
let template = self
.helper
Expand Down Expand Up @@ -2502,11 +2505,11 @@ pub fn format_template<C: Clone>(ui: &Ui, arg: &C, template: &TemplateRenderer<C
pub struct CliRunner {
tracing_subscription: TracingSubscription,
app: Command,
extra_configs: Option<config::Config>,
store_factories: Option<StoreFactories>,
working_copy_factories: Option<HashMap<String, Box<dyn WorkingCopyFactory>>>,
commit_template_extension: Option<Arc<dyn CommitTemplateLanguageExtension>>,
operation_template_extension: Option<Arc<dyn OperationTemplateLanguageExtension>>,
extra_configs: Vec<config::Config>,
store_factories: StoreFactories,
working_copy_factories: WorkingCopyFactories,
commit_template_extensions: Vec<Arc<dyn CommitTemplateLanguageExtension>>,
operation_template_extensions: Vec<Arc<dyn OperationTemplateLanguageExtension>>,
dispatch_fn: CliDispatchFn,
start_hook_fns: Vec<CliDispatchFn>,
process_global_args_fns: Vec<ProcessGlobalArgsFn>,
Expand All @@ -2525,11 +2528,11 @@ impl CliRunner {
CliRunner {
tracing_subscription,
app: crate::commands::default_app(),
extra_configs: None,
store_factories: None,
working_copy_factories: None,
commit_template_extension: None,
operation_template_extension: None,
extra_configs: vec![],
store_factories: StoreFactories::default(),
working_copy_factories: default_working_copy_factories(),
commit_template_extensions: vec![],
operation_template_extensions: vec![],
dispatch_fn: Box::new(crate::commands::run_command),
start_hook_fns: vec![],
process_global_args_fns: vec![],
Expand All @@ -2543,39 +2546,41 @@ impl CliRunner {
}

/// Adds default configs in addition to the normal defaults.
pub fn set_extra_config(mut self, extra_configs: config::Config) -> Self {
self.extra_configs = Some(extra_configs);
pub fn add_extra_config(mut self, extra_configs: config::Config) -> Self {
self.extra_configs.push(extra_configs);
self
}

/// Replaces `StoreFactories` to be used.
pub fn set_store_factories(mut self, store_factories: StoreFactories) -> Self {
self.store_factories = Some(store_factories);
/// Adds `StoreFactories` to be used.
pub fn add_store_factories(mut self, store_factories: StoreFactories) -> Self {
self.store_factories.merge(store_factories);
self
}

/// Replaces working copy factories to be used.
pub fn set_working_copy_factories(
/// Adds working copy factories to be used.
pub fn add_working_copy_factories(
mut self,
working_copy_factories: HashMap<String, Box<dyn WorkingCopyFactory>>,
working_copy_factories: WorkingCopyFactories,
) -> Self {
self.working_copy_factories = Some(working_copy_factories);
merge_factories_map(&mut self.working_copy_factories, working_copy_factories);
self
}

pub fn set_commit_template_extension(
pub fn add_commit_template_extension(
mut self,
commit_template_extension: Box<dyn CommitTemplateLanguageExtension>,
) -> Self {
self.commit_template_extension = Some(commit_template_extension.into());
self.commit_template_extensions
.push(commit_template_extension.into());
self
}

pub fn set_operation_template_extension(
pub fn add_operation_template_extension(
mut self,
operation_template_extension: Box<dyn OperationTemplateLanguageExtension>,
) -> Self {
self.operation_template_extension = Some(operation_template_extension.into());
self.operation_template_extensions
.push(operation_template_extension.into());
self
}

Expand Down Expand Up @@ -2687,9 +2692,6 @@ impl CliRunner {
}

let settings = UserSettings::from_config(config);
let working_copy_factories = self
.working_copy_factories
.unwrap_or_else(default_working_copy_factories);
let command_helper = CommandHelper {
app: self.app,
cwd,
Expand All @@ -2698,11 +2700,11 @@ impl CliRunner {
global_args: args.global_args,
settings,
layered_configs,
commit_template_extension: self.commit_template_extension,
operation_template_extension: self.operation_template_extension,
commit_template_extensions: self.commit_template_extensions,
operation_template_extensions: self.operation_template_extensions,
maybe_workspace_loader,
store_factories: self.store_factories.unwrap_or_default(),
working_copy_factories,
store_factories: self.store_factories,
working_copy_factories: self.working_copy_factories,
};
for start_hook_fn in self.start_hook_fns {
start_hook_fn(ui, &command_helper)?;
Expand All @@ -2714,10 +2716,10 @@ impl CliRunner {
#[instrument(skip(self))]
pub fn run(mut self) -> ExitCode {
let mut default_config = crate::config::default_config();
if let Some(extra_configs) = self.extra_configs.take() {
for extra_config in self.extra_configs.drain(..) {
default_config = config::Config::builder()
.add_source(default_config)
.add_source(extra_configs)
.add_source(extra_config)
.build()
.unwrap();
}
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ fn cmd_op_log(
let language = OperationTemplateLanguage::new(
repo_loader.op_store().root_operation_id(),
current_op_id,
command.operation_template_extension(),
command.operation_template_extensions(),
);
let text = match &args.template {
Some(value) => value.to_owned(),
Expand Down
7 changes: 3 additions & 4 deletions cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::cmp::max;
use std::collections::HashMap;
use std::io;
use std::rc::Rc;
use std::sync::Arc;

use itertools::Itertools as _;
use jj_lib::backend::{ChangeId, CommitId};
Expand Down Expand Up @@ -71,14 +72,12 @@ impl<'repo> CommitTemplateLanguage<'repo> {
workspace_id: &WorkspaceId,
revset_parse_context: RevsetParseContext<'repo>,
id_prefix_context: &'repo IdPrefixContext,
extension: Option<&dyn CommitTemplateLanguageExtension>,
extensions: &Vec<Arc<dyn CommitTemplateLanguageExtension>>,
) -> Self {
let mut build_fn_table = CommitTemplateBuildFnTable::builtin();
let mut cache_extensions = ExtensionsMap::empty();

// TODO: Extension methods should be refactored to be plural, to support
// multiple extensions in a dynamic load environment
if let Some(extension) = extension {
for extension in extensions {
build_fn_table.merge(extension.build_fn_table());
extension.build_cache_extensions(&mut cache_extensions);
}
Expand Down
9 changes: 5 additions & 4 deletions cli/src/operation_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use std::any::Any;
use std::collections::HashMap;
use std::io;
use std::sync::Arc;

use itertools::Itertools as _;
use jj_lib::extensions_map::ExtensionsMap;
Expand Down Expand Up @@ -51,13 +52,13 @@ impl OperationTemplateLanguage {
pub fn new(
root_op_id: &OperationId,
current_op_id: Option<&OperationId>,
extension: Option<&dyn OperationTemplateLanguageExtension>,
extensions: &Vec<Arc<dyn OperationTemplateLanguageExtension>>,
) -> Self {
let mut build_fn_table = OperationTemplateBuildFnTable::builtin();
let mut cache_extensions = ExtensionsMap::empty();
if let Some(extension) = extension {
let ext_table = extension.build_fn_table();
build_fn_table.merge(ext_table);

for extension in extensions {
build_fn_table.merge(extension.build_fn_table());
extension.build_cache_extensions(&mut cache_extensions);
}

Expand Down
28 changes: 28 additions & 0 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,15 @@ type IndexStoreFactory =
Box<dyn Fn(&UserSettings, &Path) -> Result<Box<dyn IndexStore>, BackendLoadError>>;
type SubmoduleStoreFactory = Box<dyn Fn(&UserSettings, &Path) -> Box<dyn SubmoduleStore>>;

pub fn merge_factories_map<F>(base: &mut HashMap<String, F>, ext: HashMap<String, F>) {
for (name, factory) in ext {
let copy = name.clone();
if base.insert(name, factory).is_some() {
panic!("Conflicting factory definitions for '{copy}' factory");
}
}
}

pub struct StoreFactories {
backend_factories: HashMap<String, BackendFactory>,
op_store_factories: HashMap<String, OpStoreFactory>,
Expand Down Expand Up @@ -426,6 +435,25 @@ impl StoreFactories {
}
}

pub fn merge(&mut self, ext: StoreFactories) {
let StoreFactories {
backend_factories,
op_store_factories,
op_heads_store_factories,
index_store_factories,
submodule_store_factories,
} = ext;

merge_factories_map(&mut self.backend_factories, backend_factories);
merge_factories_map(&mut self.op_store_factories, op_store_factories);
merge_factories_map(&mut self.op_heads_store_factories, op_heads_store_factories);
merge_factories_map(&mut self.index_store_factories, index_store_factories);
merge_factories_map(
&mut self.submodule_store_factories,
submodule_store_factories,
);
}

pub fn add_backend(&mut self, name: &str, factory: BackendFactory) {
self.backend_factories.insert(name.to_string(), factory);
}
Expand Down
Loading

0 comments on commit 3341bff

Please sign in to comment.