From 536c629df87b8a67eaefcc783a1e84767c779643 Mon Sep 17 00:00:00 2001 From: Arthur Grillo Date: Sat, 21 Sep 2024 01:42:50 -0300 Subject: [PATCH] cli: Explicitly add a Help command to accept the early args after it The default clap's help command doesn't have the ability to accept flags (e.g --no-pager). The recommended way[1] to solve this is to manually implement it. [1]: https://github.com/clap-rs/clap/discussions/5332 Fixes: #4501 --- CHANGELOG.md | 3 ++ cli/src/commands/help.rs | 49 ++++++++++++++++++ cli/src/commands/mod.rs | 4 ++ cli/tests/cli-reference@.md.snap | 14 ++++++ cli/tests/runner.rs | 1 + cli/tests/test_global_opts.rs | 4 ++ cli/tests/test_help_command.rs | 86 ++++++++++++++++++++++++++++++++ 7 files changed, 161 insertions(+) create mode 100644 cli/src/commands/help.rs create mode 100644 cli/tests/test_help_command.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a1d3036c9..6bd458b711 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Evaluation error of `revsets.short-prefixes` configuration is now reported. +* Help command doesn't work recursively anymore, i.e. `jj workspace help root` + doesn't work anymore. + ### Deprecations ### New features diff --git a/cli/src/commands/help.rs b/cli/src/commands/help.rs new file mode 100644 index 0000000000..364cbe7fca --- /dev/null +++ b/cli/src/commands/help.rs @@ -0,0 +1,49 @@ +// Copyright 2024 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use tracing::instrument; + +use crate::cli_util::CommandHelper; +use crate::command_error; +use crate::command_error::CommandError; +use crate::ui::Ui; + +/// Print this message or the help of the given subcommand(s) +#[derive(clap::Args, Clone, Debug)] +pub(crate) struct HelpArgs { + /// Print help for the subcommand(s) + pub(crate) command: Vec, +} + +#[instrument(skip_all)] +pub(crate) fn cmd_help( + _ui: &mut Ui, + command: &CommandHelper, + args: &HelpArgs, +) -> Result<(), CommandError> { + let mut args_to_show_help = vec![command.app().get_name()]; + args_to_show_help.extend(args.command.iter().map(|s| s.as_str())); + args_to_show_help.push("--help"); + + // TODO: `help log -- -r` will gives an cryptic error, ideally, it should state + // that the subcommand `log -r` doesn't exist. + let help_err = command + .app() + .clone() + .subcommand_required(true) + .try_get_matches_from(args_to_show_help) + .expect_err("Clap library should return a DisplayHelp error in this context"); + + Err(command_error::cli_error(help_err)) +} diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 2e96e95cba..25bf0a32f1 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -30,6 +30,7 @@ mod evolog; mod file; mod fix; mod git; +mod help; mod init; mod interdiff; mod log; @@ -71,6 +72,7 @@ use crate::command_error::CommandError; use crate::ui::Ui; #[derive(clap::Parser, Clone, Debug)] +#[command(disable_help_subcommand = true)] enum Command { Abandon(abandon::AbandonArgs), Backout(backout::BackoutArgs), @@ -110,6 +112,7 @@ enum Command { Fix(fix::FixArgs), #[command(subcommand)] Git(git::GitCommand), + Help(help::HelpArgs), Init(init::InitArgs), Interdiff(interdiff::InterdiffArgs), Log(log::LogArgs), @@ -217,6 +220,7 @@ pub fn run_command(ui: &mut Ui, command_helper: &CommandHelper) -> Result<(), Co } Command::Fix(args) => fix::cmd_fix(ui, command_helper, args), Command::Git(args) => git::cmd_git(ui, command_helper, args), + Command::Help(args) => help::cmd_help(ui, command_helper, args), Command::Init(args) => init::cmd_init(ui, command_helper, args), Command::Interdiff(args) => interdiff::cmd_interdiff(ui, command_helper, args), Command::Log(args) => log::cmd_log(ui, command_helper, args), diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index e9c7f719a7..b1cbbecedf 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -56,6 +56,7 @@ This document contains the help content for the `jj` command-line program. * [`jj git remote remove`↴](#jj-git-remote-remove) * [`jj git remote rename`↴](#jj-git-remote-rename) * [`jj git remote set-url`↴](#jj-git-remote-set-url) +* [`jj help`↴](#jj-help) * [`jj init`↴](#jj-init) * [`jj interdiff`↴](#jj-interdiff) * [`jj log`↴](#jj-log) @@ -126,6 +127,7 @@ To get started, see the tutorial at https://martinvonz.github.io/jj/latest/tutor * `file` — File operations * `fix` — Update files with formatting fixes or other changes * `git` — Commands for working with Git remotes and the underlying Git repo +* `help` — Print this message or the help of the given subcommand(s) * `init` — Create a new repo in the given directory * `interdiff` — Compare the changes of two commits * `log` — Show revision history @@ -1164,6 +1166,18 @@ Set the URL of a Git remote +## `jj help` + +Print this message or the help of the given subcommand(s) + +**Usage:** `jj help [COMMAND]...` + +###### **Arguments:** + +* `` — Print help for the subcommand(s) + + + ## `jj init` Create a new repo in the given directory diff --git a/cli/tests/runner.rs b/cli/tests/runner.rs index 90975b2312..7ecc1d7b98 100644 --- a/cli/tests/runner.rs +++ b/cli/tests/runner.rs @@ -44,6 +44,7 @@ mod test_git_remotes; mod test_git_submodule; mod test_gitignores; mod test_global_opts; +mod test_help_command; mod test_immutable_commits; mod test_init_command; mod test_interdiff_command; diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index 1ea6246812..c3eaf146d6 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -555,6 +555,10 @@ fn test_early_args() { let stdout = test_env.jj_cmd_success(test_env.env_root(), &["--color=always", "help"]); insta::assert_snapshot!(stdout.lines().find(|l| l.contains("Commands:")).unwrap(), @"Commands:"); + // Check that early args are accepted after the help command + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["help", "--color=always"]); + insta::assert_snapshot!(stdout.lines().find(|l| l.contains("Commands:")).unwrap(), @"Commands:"); + // Early args are parsed with clap's ignore_errors(), but there is a known // bug that causes defaults to be unpopulated. Test that the early args are // tolerant of this bug and don't cause a crash. diff --git a/cli/tests/test_help_command.rs b/cli/tests/test_help_command.rs new file mode 100644 index 0000000000..8c8d0d12f6 --- /dev/null +++ b/cli/tests/test_help_command.rs @@ -0,0 +1,86 @@ +// Copyright 2024 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::common::TestEnvironment; + +#[test] +fn test_help() { + let test_env = TestEnvironment::default(); + + let help_cmd_stdout = test_env.jj_cmd_success(test_env.env_root(), &["help"]); + // The help command output should be equal to the long --help flag + let help_flag_stdout = test_env.jj_cmd_success(test_env.env_root(), &["--help"]); + assert_eq!(help_cmd_stdout, help_flag_stdout); + + // Help command should work with commands + let help_cmd_stdout = test_env.jj_cmd_success(test_env.env_root(), &["help", "log"]); + let help_flag_stdout = test_env.jj_cmd_success(test_env.env_root(), &["log", "--help"]); + assert_eq!(help_cmd_stdout, help_flag_stdout); + + // Help command should work with subcommands + let help_cmd_stdout = + test_env.jj_cmd_success(test_env.env_root(), &["help", "workspace", "root"]); + let help_flag_stdout = + test_env.jj_cmd_success(test_env.env_root(), &["workspace", "root", "--help"]); + assert_eq!(help_cmd_stdout, help_flag_stdout); + + // Help command should not work recursively + let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["workspace", "help", "root"]); + insta::assert_snapshot!(stderr, @r#" + error: unrecognized subcommand 'help' + + Usage: jj workspace [OPTIONS] + + For more information, try '--help'. + "#); + + let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["workspace", "add", "help"]); + insta::assert_snapshot!(stderr, @r#" + Error: There is no jj repo in "." + "#); + + let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["new", "help", "main"]); + insta::assert_snapshot!(stderr, @r#" + Error: There is no jj repo in "." + "#); + + // Help command should output the same as --help for nonexistent commands + let help_cmd_stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["help", "nonexistent"]); + let help_flag_stderr = + test_env.jj_cmd_cli_error(test_env.env_root(), &["nonexistent", "--help"]); + assert_eq!(help_cmd_stderr, help_flag_stderr); + + // Some edge cases + let help_cmd_stdout = test_env.jj_cmd_success(test_env.env_root(), &["help", "help"]); + let help_flag_stdout = test_env.jj_cmd_success(test_env.env_root(), &["help", "--help"]); + assert_eq!(help_cmd_stdout, help_flag_stdout); + + let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["help", "unknown"]); + insta::assert_snapshot!(stderr, @r#" + error: unrecognized subcommand 'unknown' + + tip: a similar subcommand exists: 'undo' + + Usage: jj [OPTIONS] + + For more information, try '--help'. + "#); + + let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["help", "log", "--", "-r"]); + insta::assert_snapshot!(stderr, @r#" + error: a value is required for '--revisions ' but none was supplied + + For more information, try '--help'. + "#); +}