From cf7342b56c0c7cf4c18e3fd5d52b98f910609c81 Mon Sep 17 00:00:00 2001 From: Remo Senekowitsch Date: Sat, 16 Nov 2024 18:42:26 +0100 Subject: [PATCH] completion: teach config about keys There are two known limitations right now: - Only statically known keys are suggested. - Keys that the user did not set are still suggested for `jj config get`. Running that suggestion may result in an error. The error message will be appropriate though and there is some value in letting the user know that this config value theoretically exists. Some users may try to explore what configurations are available via the completions. --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/src/commands/config/get.rs | 4 +- cli/src/commands/config/list.rs | 3 ++ cli/src/commands/config/set.rs | 4 +- cli/src/commands/config/unset.rs | 4 +- cli/src/complete.rs | 67 ++++++++++++++++++++++++++++++++ 7 files changed, 81 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e0bccea695e..2490ba39391 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1845,6 +1845,7 @@ dependencies = [ "sapling-renderdag", "scm-record", "serde", + "serde_json", "slab", "strsim", "tempfile", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4ec287923f6..8d871a0865f 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -80,6 +80,7 @@ rpassword = { workspace = true } sapling-renderdag = { workspace = true } scm-record = { workspace = true } serde = { workspace = true } +serde_json.workspace = true slab = { workspace = true } strsim = { workspace = true } tempfile = { workspace = true } diff --git a/cli/src/commands/config/get.rs b/cli/src/commands/config/get.rs index abeb4eebd29..70184cf78ef 100644 --- a/cli/src/commands/config/get.rs +++ b/cli/src/commands/config/get.rs @@ -14,11 +14,13 @@ use std::io::Write as _; +use clap_complete::ArgValueCandidates; use tracing::instrument; use crate::cli_util::CommandHelper; use crate::command_error::config_error; use crate::command_error::CommandError; +use crate::complete; use crate::config::ConfigNamePathBuf; use crate::ui::Ui; @@ -34,7 +36,7 @@ use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] #[command(verbatim_doc_comment)] pub struct ConfigGetArgs { - #[arg(required = true)] + #[arg(required = true, add = ArgValueCandidates::new(complete::leaf_config_keys))] name: ConfigNamePathBuf, } diff --git a/cli/src/commands/config/list.rs b/cli/src/commands/config/list.rs index 8308171af93..e4501a40fbc 100644 --- a/cli/src/commands/config/list.rs +++ b/cli/src/commands/config/list.rs @@ -12,11 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. +use clap_complete::ArgValueCandidates; use tracing::instrument; use super::ConfigLevelArgs; use crate::cli_util::CommandHelper; use crate::command_error::CommandError; +use crate::complete; use crate::config::to_toml_value; use crate::config::AnnotatedValue; use crate::config::ConfigNamePathBuf; @@ -31,6 +33,7 @@ use crate::ui::Ui; #[command(mut_group("config_level", |g| g.required(false)))] pub struct ConfigListArgs { /// An optional name of a specific config option to look up. + #[arg(add = ArgValueCandidates::new(complete::config_keys))] pub name: Option, /// Whether to explicitly include built-in default values in the list. #[arg(long, conflicts_with = "config_level")] diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index e4a9106edc7..ff4f816c5f6 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -14,6 +14,7 @@ use std::io; +use clap_complete::ArgValueCandidates; use jj_lib::commit::Commit; use jj_lib::repo::Repo; use tracing::instrument; @@ -24,6 +25,7 @@ use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::user_error; use crate::command_error::CommandError; +use crate::complete; use crate::config::parse_toml_value_or_bare_string; use crate::config::write_config_value_to_file; use crate::config::ConfigNamePathBuf; @@ -32,7 +34,7 @@ use crate::ui::Ui; /// Update config file to set the given option to a given value. #[derive(clap::Args, Clone, Debug)] pub struct ConfigSetArgs { - #[arg(required = true)] + #[arg(required = true, add = ArgValueCandidates::new(complete::leaf_config_keys))] name: ConfigNamePathBuf, #[arg(required = true)] value: String, diff --git a/cli/src/commands/config/unset.rs b/cli/src/commands/config/unset.rs index 46a8446f399..3718732af65 100644 --- a/cli/src/commands/config/unset.rs +++ b/cli/src/commands/config/unset.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use clap_complete::ArgValueCandidates; use tracing::instrument; use super::ConfigLevelArgs; @@ -19,6 +20,7 @@ use crate::cli_util::get_new_config_file_path; use crate::cli_util::CommandHelper; use crate::command_error::user_error; use crate::command_error::CommandError; +use crate::complete; use crate::config::remove_config_value_from_file; use crate::config::ConfigNamePathBuf; use crate::ui::Ui; @@ -26,7 +28,7 @@ use crate::ui::Ui; /// Update config file to unset the given option. #[derive(clap::Args, Clone, Debug)] pub struct ConfigUnsetArgs { - #[arg(required = true)] + #[arg(required = true, add = ArgValueCandidates::new(complete::config_keys))] name: ConfigNamePathBuf, #[command(flatten)] level: ConfigLevelArgs, diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 997c8908583..fd9c8926a4c 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -271,6 +271,73 @@ pub fn operations() -> Vec { }) } +fn config_keys_rec( + prefix: &str, + properties: &serde_json::Map, + acc: &mut Vec, + only_leaves: bool, +) { + for (key, value) in properties { + if key == "type" { + // used by "additionalProperties", ignore + continue; + } + let owned_key; + let key = if key.contains('(') { + // Parenthesis can occur in revset alises, e.g. "immutable_head()". + // These must be escaped. + owned_key = format!("'{key}'"); + &owned_key + } else { + key + }; + let value = value.as_object().unwrap(); + match value.get("type").and_then(|v| v.as_str()) { + Some("object") => { + if !only_leaves { + acc.push(CompletionCandidate::new(format!("{prefix}{key}"))); + } + let Some(properties) = value.get("properties") else { + continue; + }; + let properties = properties.as_object().unwrap(); + config_keys_rec(&format!("{prefix}{key}."), properties, acc, only_leaves); + } + _ => { + let help = value + .get("description") + .map(|desc| desc.as_str().unwrap().to_string().into()); + acc.push(CompletionCandidate::new(format!("{prefix}{key}")).help(help)); + } + } + } +} + +fn config_keys_impl(only_leaves: bool) -> Vec { + let schema = include_str!("config-schema.json"); + let schema: serde_json::Value = serde_json::from_str(schema).unwrap(); + let schema = schema.as_object().unwrap(); + let properties = schema["properties"].as_object().unwrap(); + + let mut candidates = Vec::new(); + config_keys_rec("", properties, &mut candidates, only_leaves); + candidates +} + +pub fn config_keys() -> Vec { + config_keys_impl(false) +} + +pub fn leaf_config_keys() -> Vec { + config_keys_impl(true) +} + +#[test] +fn test_config_keys() { + // Just make sure the schema is parsed without failure. + let _ = config_keys(); +} + /// Shell out to jj during dynamic completion generation /// /// In case of errors, print them and early return an empty vector.