Skip to content

Commit

Permalink
cli: Explicitly add a Help command to accept the early args after it
Browse files Browse the repository at this point in the history
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]: clap-rs/clap#5332

Fixes: #4501
  • Loading branch information
Grillo-0 committed Oct 11, 2024
1 parent d59591f commit 2844cec
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions cli/src/commands/help.rs
Original file line number Diff line number Diff line change
@@ -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<String>,
}

#[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))
}
4 changes: 4 additions & 0 deletions cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ mod evolog;
mod file;
mod fix;
mod git;
mod help;
mod init;
mod interdiff;
mod log;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
14 changes: 14 additions & 0 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:**
* `<COMMAND>` — Print help for the subcommand(s)
## `jj init`
Create a new repo in the given directory
Expand Down
1 change: 1 addition & 0 deletions cli/tests/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions cli/tests/test_global_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
86 changes: 86 additions & 0 deletions cli/tests/test_help_command.rs
Original file line number Diff line number Diff line change
@@ -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] <COMMAND>
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] <COMMAND>
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 <REVISIONS>' but none was supplied
For more information, try '--help'.
"#);
}

0 comments on commit 2844cec

Please sign in to comment.