From a6d80090971578ba45f35deed82eca5238e847d4 Mon Sep 17 00:00:00 2001 From: Essien Ita Essien <34972+essiene@users.noreply.github.com> Date: Thu, 1 Aug 2024 20:17:16 +0100 Subject: [PATCH] Define `builtin_immutable_heads()` as a default revset alias. * Add `builtin_immutable_heads()` in the `revsets.toml`. * Redefine `immutable_heads()` in terms of `builtin_immutable_heads()` * Warn if user redefines `builtin_immutable_heads()`, `mutable()` or `immutable()`. * Update module constant in revset_util.rs from BUILTIN_IMMUTABLE_HEADS to USER_IMMUTABLE_HEADS to avoid confusion since it points at `immutable_heads()` **and** we now have a revset-alias literally named `builtin_immutable_heads()`. * Add unittest * Update CHANGELOG * Update documentation. Fixes: #4162 --- CHANGELOG.md | 4 ++++ cli/src/config/revsets.toml | 3 ++- cli/src/revset_util.rs | 35 +++++++++++++++++++++++++++---- cli/tests/test_builtin_aliases.rs | 21 +++++++++++++++++++ docs/revsets.md | 21 ++++++++++++------- 5 files changed, 71 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 608a329c43..610d20ab38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,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 should 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. diff --git a/cli/src/config/revsets.toml b/cli/src/config/revsets.toml index f91b393093..253e16778b 100644 --- a/cli/src/config/revsets.toml +++ b/cli/src/config/revsets.toml @@ -18,6 +18,7 @@ latest( ) ''' -'immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()' +'builtin_immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()' +'immutable_heads()' = 'builtin_immutable_heads()' 'immutable()' = '::(immutable_heads() | root())' 'mutable()' = '~immutable()' diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index 96d3adeb80..96dfb6d066 100644 --- a/cli/src/revset_util.rs +++ b/cli/src/revset_util.rs @@ -31,12 +31,12 @@ 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; -const BUILTIN_IMMUTABLE_HEADS: &str = "immutable_heads"; +const USER_IMMUTABLE_HEADS: &str = "immutable_heads"; #[derive(Debug, Error)] pub enum UserRevsetEvaluationError { @@ -107,6 +107,30 @@ 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 => { + let checked_mutability_builtins = + ["mutable()", "immutable()", "builtin_immutable_heads()"]; + + if checked_mutability_builtins.contains(&name) { + writeln!( + ui.warning_default(), + "Redefining `revset-aliases.{name}` is not recommended; redefine \ + `immutable_heads()` instead", + )?; + } + } + } + + Ok(()) +} + pub fn load_revset_aliases( ui: &Ui, layered_configs: &LayeredConfigs, @@ -115,17 +139,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(), @@ -167,7 +194,7 @@ pub fn parse_immutable_heads_expression( ) -> Result, RevsetParseError> { let (_, _, immutable_heads_str) = context .aliases_map() - .get_function(BUILTIN_IMMUTABLE_HEADS, 0) + .get_function(USER_IMMUTABLE_HEADS, 0) .unwrap(); let heads = revset::parse(immutable_heads_str, context)?; Ok(heads.union(&RevsetExpression::root())) diff --git a/cli/tests/test_builtin_aliases.rs b/cli/tests/test_builtin_aliases.rs index 30dbe48dc6..52400c8501 100644 --- a/cli/tests/test_builtin_aliases.rs +++ b/cli/tests/test_builtin_aliases.rs @@ -136,3 +136,24 @@ 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()' = '@'"#); + test_env.add_config(r#"revset-aliases.'mutable()' = '@'"#); + test_env.add_config(r#"revset-aliases.'immutable()' = '@'"#); + + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-r", "trunk()"]); + insta::assert_snapshot!(stdout, @r###" + ○ xtvrqkyv test.user@example.com 2001-02-03 08:05:08 main d13ecdbd + │ (empty) description 1 + ~ + "###); + insta::assert_snapshot!(stderr, @r###" + Warning: Redefining `revset-aliases.builtin_immutable_heads()` is not recommended; redefine `immutable_heads()` instead + Warning: Redefining `revset-aliases.immutable()` is not recommended; redefine `immutable_heads()` instead + Warning: Redefining `revset-aliases.mutable()` is not recommended; redefine `immutable_heads()` instead + "###); +} diff --git a/docs/revsets.md b/docs/revsets.md index 2b99e9689e..421fe02b76 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -426,19 +426,24 @@ for a comprehensive list. 'trunk()' = 'your-branch@your-remote' ``` +* `builtin_immutable_heads()`: Resolves to `trunk() | tags() | untracked_remote_branches()`. + It is used as the default definition for `immutable_heads()` below. it is not + recommended to redefined this alias. Prefer to redefine `immutable_heads()` instead. + * `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 - *not* change whether a commit is immutable. To do that, edit - `immutable_heads()`. + equivalent to `::(immutable_heads() | root())`. It is not recommended to redefine + this alias. Note that modifying this will *not* change whether a commit is immutable. + To do that, edit `immutable_heads()`. * `mutable()`: The set of commits that `jj` treats as mutable. This is - equivalent to `~immutable()`. Note that modifying this will - *not* change whether a commit is immutable. To do that, edit - `immutable_heads()`. + equivalent to `~immutable()`. It is not recommended to redefined this alias. + Note that modifying this will *not* change whether a commit is immutable. + To do that, edit `immutable_heads()`. ## The `all:` modifier