Skip to content

Commit

Permalink
Add a builtin overrides config source.
Browse files Browse the repository at this point in the history
* The builtin overrides config source is backed by
  `config/builtins.toml`, but is sourced last.
* This will take precedence over everything else.
* Define `builtin_immutable_heads()` in the `builtins.toml`
* Warn if user redefines `builtin_`, that these may be ignored.
* Redefine `immutable_heads()` in terms of `builtin_immutable_heads()`
* Add unittests
* Update CHANGELOG and documentation.
  • Loading branch information
essiene committed Aug 11, 2024
1 parent c9e147c commit 89907b3
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### New features

* Define `immutable_heads()` revset alias in terms of a new `builtin_immutable_heads()`.
This enables users to redefine `immutable_heads()` as they wish, but still
have `builtin_immutable_heads()` which can not be redefined.

* External diff tools can now be configured to invoke the tool on each file
individually instead of being passed a directory by setting
`merge-tools.$TOOL.diff-invocation-mode="file-by-file"` in config.toml.
Expand Down
25 changes: 22 additions & 3 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ pub enum ConfigSource {
User,
Repo,
CommandArg,
Builtin,
}

#[derive(Clone, Debug, PartialEq)]
Expand All @@ -208,6 +209,7 @@ pub struct AnnotatedValue {
/// 5. TODO: Workspace config `.jj/config.toml`
/// 6. Override environment variables
/// 7. Command-line arguments `--config-toml`
/// 8. Builtins
#[derive(Clone, Debug)]
pub struct LayeredConfigs {
default: config::Config,
Expand All @@ -216,6 +218,7 @@ pub struct LayeredConfigs {
repo: Option<config::Config>,
env_overrides: config::Config,
arg_overrides: Option<config::Config>,
builtin_overrides: config::Config,
}

impl LayeredConfigs {
Expand All @@ -228,6 +231,7 @@ impl LayeredConfigs {
repo: None,
env_overrides: env_overrides(),
arg_overrides: None,
builtin_overrides: builtin_overrides(),
}
}

Expand Down Expand Up @@ -284,6 +288,7 @@ impl LayeredConfigs {
(ConfigSource::Repo, self.repo.as_ref()),
(ConfigSource::Env, Some(&self.env_overrides)),
(ConfigSource::CommandArg, self.arg_overrides.as_ref()),
(ConfigSource::Builtin, Some(&self.builtin_overrides)),
];
config_sources
.into_iter()
Expand Down Expand Up @@ -499,6 +504,17 @@ pub fn default_config() -> config::Config {
builder.build().unwrap()
}

/// Builtin overrides that override all other values.
pub fn builtin_overrides() -> config::Config {
config::Config::builder()
.add_source(config::File::from_str(
include_str!("config/builtins.toml"),
config::FileFormat::Toml,
))
.build()
.unwrap()
}

/// Environment variables that override config values
fn env_overrides() -> config::Config {
let mut builder = config::Config::builder();
Expand Down Expand Up @@ -885,8 +901,9 @@ mod tests {
env_base: empty_config.to_owned(),
user: None,
repo: None,
env_overrides: empty_config,
env_overrides: empty_config.to_owned(),
arg_overrides: None,
builtin_overrides: empty_config.to_owned(),
};
assert_eq!(
layered_configs
Expand Down Expand Up @@ -916,8 +933,9 @@ mod tests {
env_base: env_base_config,
user: None,
repo: Some(repo_config),
env_overrides: empty_config,
env_overrides: empty_config.to_owned(),
arg_overrides: None,
builtin_overrides: empty_config.to_owned(),
};
// Note: "email" is alphabetized, before "name" from same layer.
insta::assert_debug_snapshot!(
Expand Down Expand Up @@ -1039,8 +1057,9 @@ mod tests {
env_base: empty_config.to_owned(),
user: Some(user_config),
repo: Some(repo_config),
env_overrides: empty_config,
env_overrides: empty_config.to_owned(),
arg_overrides: None,
builtin_overrides: empty_config.to_owned(),
};
insta::assert_debug_snapshot!(
layered_configs
Expand Down
3 changes: 3 additions & 0 deletions cli/src/config/builtins.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[revset-aliases]
'builtin_immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()'

2 changes: 1 addition & 1 deletion cli/src/config/revsets.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ latest(
)
'''

'immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()'
'immutable_heads()' = 'builtin_immutable_heads()'
'immutable()' = '::(immutable_heads() | root())'
'mutable()' = '~immutable()'
33 changes: 31 additions & 2 deletions cli/src/revset_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use jj_lib::settings::ConfigResultExt as _;
use thiserror::Error;

use crate::command_error::{user_error, CommandError};
use crate::config::LayeredConfigs;
use crate::config::{ConfigSource, LayeredConfigs};
use crate::formatter::Formatter;
use crate::templater::TemplateRenderer;
use crate::ui::Ui;
Expand Down Expand Up @@ -107,6 +107,32 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
}
}

fn warn_user_redefined_builtin(
ui: &Ui,
source: &ConfigSource,
name: &str,
) -> Result<(), CommandError> {
match source {
ConfigSource::Default
| ConfigSource::Env
| ConfigSource::User
| ConfigSource::Repo
| ConfigSource::CommandArg => {
if name.starts_with("builtin") {
writeln!(
ui.warning_default(),
"`revset-aliases.{}` may be overridden; aliases starting with `builtin` are \
reserved",
name
)?;
}
}
ConfigSource::Builtin => (),
}

Ok(())
}

pub fn load_revset_aliases(
ui: &Ui,
layered_configs: &LayeredConfigs,
Expand All @@ -115,17 +141,20 @@ pub fn load_revset_aliases(
let mut aliases_map = RevsetAliasesMap::new();
// Load from all config layers in order. 'f(x)' in default layer should be
// overridden by 'f(a)' in user.
for (_, config) in layered_configs.sources() {
for (source, config) in layered_configs.sources() {
let table = if let Some(table) = config.get_table(TABLE_KEY).optional()? {
table
} else {
continue;
};
for (decl, value) in table.into_iter().sorted_by(|a, b| a.0.cmp(&b.0)) {
warn_user_redefined_builtin(ui, &source, &decl)?;

let r = value
.into_string()
.map_err(|e| e.to_string())
.and_then(|v| aliases_map.insert(&decl, v).map_err(|e| e.to_string()));

if let Err(s) = r {
writeln!(
ui.warning_default(),
Expand Down
17 changes: 17 additions & 0 deletions cli/tests/test_builtin_aliases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,20 @@ fn test_builtin_alias_trunk_no_match_only_exact() {
insta::assert_snapshot!(stderr, @r###"
"###);
}

#[test]
fn test_builtin_user_redefines_builtin_immutable_heads() {
let (test_env, workspace_root) = set_up("main");

test_env.add_config(r#"revset-aliases.'builtin_immutable_heads()' = '@'"#);

let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-r", "trunk()"]);
insta::assert_snapshot!(stdout, @r###"
◆ xtvrqkyv [email protected] 2001-02-03 08:05:08 main d13ecdbd
│ (empty) description 1
~
"###);
insta::assert_snapshot!(stderr, @r###"
Warning: `revset-aliases.builtin_immutable_heads()` may be overridden; aliases starting with `builtin` are reserved
"###);
}
14 changes: 10 additions & 4 deletions docs/revsets.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,11 @@ For example:

### Built-in Aliases

The following aliases are built-in and used for certain operations. These functions
The following aliases are built-in and used for certain operations. Most of these functions
are defined as aliases in order to allow you to overwrite them as needed.
See [revsets.toml](https://github.com/martinvonz/jj/blob/main/cli/src/config/revsets.toml)
for a comprehensive list.
for a comprehensive list. There are however [builtins](https://github.com/martinvonz/jj/blob/main/cli/src/config/builtins.toml)
that can not be overridden nor redefined.

* `trunk()`: Resolves to the head commit for the trunk branch of the remote
named `origin` or `upstream`. The branches `main`, `master`, and `trunk` are
Expand All @@ -426,9 +427,14 @@ for a comprehensive list.
'trunk()' = 'your-branch@your-remote'
```

* `builtin_immutable_heads()`: Resolves to `trunk() | tags() | untracked_remote_branches()`.
This builtin **can not** be overridden. It is used as the default definition for
`immutable_heads()` below.

* `immutable_heads()`: Resolves to `trunk() | tags() |
untracked_remote_branches()` by default. See
[here](config.md#set-of-immutable-commits) for details.
untracked_remote_branches()` by default. It is actually defined as `builtin_immutable_heads()`,
and can be overridden as required.
See [here](config.md#set-of-immutable-commits) for details.

* `immutable()`: The set of commits that `jj` treats as immutable. This is
equivalent to `::(immutable_heads() | root())`. Note that modifying this will
Expand Down

0 comments on commit 89907b3

Please sign in to comment.