Skip to content

Commit

Permalink
completion: teach config about keys
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
senekor committed Nov 18, 2024
1 parent 1a121ea commit ecd64aa
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 6 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
4 changes: 3 additions & 1 deletion cli/src/commands/config/get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
}

Expand Down
3 changes: 3 additions & 0 deletions cli/src/commands/config/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<ConfigNamePathBuf>,
/// Whether to explicitly include built-in default values in the list.
#[arg(long, conflicts_with = "config_level")]
Expand Down
4 changes: 3 additions & 1 deletion cli/src/commands/config/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use std::io;

use clap_complete::ArgValueCandidates;
use jj_lib::commit::Commit;
use jj_lib::repo::Repo;
use tracing::instrument;
Expand All @@ -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;
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion cli/src/commands/config/unset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,23 @@
// 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::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;

/// 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::leaf_config_keys))]
name: ConfigNamePathBuf,
#[command(flatten)]
level: ConfigLevelArgs,
Expand Down
5 changes: 2 additions & 3 deletions cli/src/commands/util/config_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::io::Write as _;

use crate::cli_util::CommandHelper;
use crate::command_error::CommandError;
use crate::config::CONFIG_SCHEMA;
use crate::ui::Ui;

/// Print the JSON schema for the jj TOML config format.
Expand All @@ -27,8 +28,6 @@ pub fn cmd_util_config_schema(
_command: &CommandHelper,
_args: &UtilConfigSchemaArgs,
) -> Result<(), CommandError> {
// TODO(#879): Consider generating entire schema dynamically vs. static file.
let buf = include_bytes!("../../config-schema.json");
ui.stdout().write_all(buf)?;
ui.stdout().write_all(CONFIG_SCHEMA.as_bytes())?;
Ok(())
}
73 changes: 73 additions & 0 deletions cli/src/complete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ use crate::cli_util::GlobalArgs;
use crate::command_error::user_error;
use crate::command_error::CommandError;
use crate::config::default_config;
use crate::config::ConfigNamePathBuf;
use crate::config::LayeredConfigs;
use crate::config::CONFIG_SCHEMA;
use crate::ui::Ui;

const BOOKMARK_HELP_TEMPLATE: &str = r#"
Expand Down Expand Up @@ -292,6 +294,66 @@ pub fn workspaces() -> Vec<CompletionCandidate> {
})
}

fn config_keys_rec(
prefix: ConfigNamePathBuf,
properties: &serde_json::Map<String, serde_json::Value>,
acc: &mut Vec<CompletionCandidate>,
only_leaves: bool,
) {
for (key, value) in properties {
let mut prefix = prefix.clone();
prefix.push(key);

let value = value.as_object().unwrap();
match value.get("type").and_then(|v| v.as_str()) {
Some("object") => {
if !only_leaves {
let help = value
.get("description")
.map(|desc| desc.as_str().unwrap().to_string().into());
let escaped_key = prefix.to_string();
acc.push(CompletionCandidate::new(escaped_key).help(help));
}
let Some(properties) = value.get("properties") else {
continue;
};
let properties = properties.as_object().unwrap();
config_keys_rec(prefix, properties, acc, only_leaves);
}
_ => {
let help = value
.get("description")
.map(|desc| desc.as_str().unwrap().to_string().into());
let escaped_key = prefix.to_string();
acc.push(CompletionCandidate::new(escaped_key).help(help));
}
}
}
}

fn config_keys_impl(only_leaves: bool) -> Vec<CompletionCandidate> {
let schema: serde_json::Value = serde_json::from_str(CONFIG_SCHEMA).unwrap();
let schema = schema.as_object().unwrap();
let properties = schema["properties"].as_object().unwrap();

let mut candidates = Vec::new();
config_keys_rec(
ConfigNamePathBuf::root(),
properties,
&mut candidates,
only_leaves,
);
candidates
}

pub fn config_keys() -> Vec<CompletionCandidate> {
config_keys_impl(false)
}

pub fn leaf_config_keys() -> Vec<CompletionCandidate> {
config_keys_impl(true)
}

/// Shell out to jj during dynamic completion generation
///
/// In case of errors, print them and early return an empty vector.
Expand Down Expand Up @@ -402,3 +464,14 @@ fn get_jj_command() -> Result<(std::process::Command, Config), CommandError> {

Ok((cmd, config))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_config_keys() {
// Just make sure the schema is parsed without failure.
let _ = config_keys();
}
}
3 changes: 3 additions & 0 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ use crate::command_error::user_error;
use crate::command_error::user_error_with_message;
use crate::command_error::CommandError;

// TODO(#879): Consider generating entire schema dynamically vs. static file.
pub const CONFIG_SCHEMA: &str = include_str!("config-schema.json");

/// Parses a TOML value expression. Interprets the given value as string if it
/// can't be parsed.
pub fn parse_toml_value_or_bare_string(value_str: &str) -> toml_edit::Value {
Expand Down
22 changes: 22 additions & 0 deletions cli/tests/test_completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,25 @@ fn test_workspaces() {
default initial
");
}

#[test]
fn test_config() {
let mut test_env = TestEnvironment::default();
test_env.add_env_var("COMPLETE", "fish");
let dir = test_env.env_root();

let stdout = test_env.jj_cmd_success(dir, &["--", "jj", "config", "get", "c"]);
insta::assert_snapshot!(stdout, @r"
core.fsmonitor Whether to use an external filesystem monitor, useful for large repos
core.watchman.register_snapshot_trigger Whether to use triggers to monitor for changes in the background.
");

let stdout = test_env.jj_cmd_success(dir, &["--", "jj", "config", "list", "c"]);
insta::assert_snapshot!(stdout, @r"
colors Mapping from jj formatter labels to colors
core
core.fsmonitor Whether to use an external filesystem monitor, useful for large repos
core.watchman
core.watchman.register_snapshot_trigger Whether to use triggers to monitor for changes in the background.
");
}

0 comments on commit ecd64aa

Please sign in to comment.