feat(dispatch): walk subcommands and detect revset flag from help

Two real failures motivated this:

- `jjf bm main` (where `bm` is a user alias for `bookmark move`) ran
  `jj bm main -r <id>` — but bookmark move uses `--to`, not `-r`.
- `jjf tag set v0.2.0` errored with "doesn't take revisions" because
  jjf only inspected `jj tag --help` (parent) instead of walking
  into the `set` leaf, which does take `-r`.

Replace the boolean `supports_revisions()` (a substring match on
"REVSET") with `RevsetTarget { leaf, passthrough, flag }`:

- Walks multi-level subcommands by parsing each parent's `Commands:`
  block — including `[aliases: …]` annotations, so `jjf tag s` walks
  to `tag set`. Aliases like `bm` work transparently because `jj bm
  --help` already emits the resolved leaf's help.
- Parses the leaf's `Options:` block and picks the revset flag in
  priority order: `-r`/`--revision[s]` first, then `-t, --to`.
  `--from` is intentionally not a fallback (different semantics from
  "the revision the picker chose").
- Bypasses the picker entirely when the user already passed a revset
  flag in their args, so `jjf describe -r @-` runs verbatim instead
  of double-injecting `-r`.

`exec` and `command_line` now take `&RevsetTarget` and emit the
resolved flag; signatures thread through `tui.rs` and `main.rs`.

New unit tests cover the help parsers and command-line construction
against captured fixtures of `jj tag set --help`, `jj bookmark move
--help`, and `jj tag --help`.
This commit is contained in:
swaits committed 2026-05-04 12:58:14 -06:00
1 parent 1c4ae09646
commit 6ce9cb2b17
3 files changed
+471 -50

No files matched your search

+417 -21
View File
@@ -265,42 +265,294 @@ fn parse_show_summary(bytes: &[u8]) -> PreviewParts {
PreviewParts { description, files }
}
pub fn supports_revisions(subcommand: &str) -> Result<bool> {
/// Which flag a leaf jj subcommand uses to accept a revset, picked in
/// priority order: `-r`/`--revision[s]` first, then `-t, --to`. `--from` is
/// intentionally not a fallback — its semantics ("filter source revisions")
/// differ enough from "the revision the picker chose" that silently injecting
/// it would surprise users.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RevsetFlag {
Revision,
To,
}
impl RevsetFlag {
pub fn as_str(self) -> &'static str {
match self {
RevsetFlag::Revision => "-r",
RevsetFlag::To => "--to",
}
}
}
/// A jj invocation jjf has analyzed and is ready to dispatch on. `leaf` is
/// the resolved subcommand path (preserving the user's tokens, including
/// aliases like `bm` — jj resolves them again at exec time). `passthrough`
/// is everything after the leaf. `flag` is `None` when the leaf accepts no
/// revset; in that case jjf refuses to run the picker.
#[derive(Debug, Clone)]
pub struct RevsetTarget {
pub leaf: Vec<String>,
pub passthrough: Vec<String>,
pub flag: Option<RevsetFlag>,
}
pub fn resolve_target(args: &[String]) -> Result<RevsetTarget> {
debug_assert!(!args.is_empty(), "resolve_target requires at least one arg");
let first_help = match run_help(std::slice::from_ref(&args[0]))? {
Some(h) => h,
None => {
// Unknown subcommand or jj help failed for some reason. Fall
// through optimistically so jj surfaces its own error message at
// exec time.
return Ok(RevsetTarget {
leaf: vec![args[0].clone()],
passthrough: args[1..].to_vec(),
flag: Some(RevsetFlag::Revision),
});
}
};
let (leaf, leaf_help) = resolve_leaf(args, first_help)?;
let passthrough = args[leaf.len()..].to_vec();
let flag = parse_revset_flag(&leaf_help);
Ok(RevsetTarget {
leaf,
passthrough,
flag,
})
}
fn resolve_leaf(args: &[String], first_help: String) -> Result<(Vec<String>, String)> {
let mut leaf = vec![args[0].clone()];
let mut help = first_help;
for arg in &args[1..] {
if arg.starts_with('-') {
break;
}
let children = parse_commands_section(&help);
if !children.contains(arg.as_str()) {
break;
}
let mut candidate = leaf.clone();
candidate.push(arg.clone());
match run_help(&candidate)? {
Some(next_help) => {
leaf = candidate;
help = next_help;
}
None => break,
}
}
Ok((leaf, help))
}
fn run_help(path: &[String]) -> Result<Option<String>> {
let output = Command::new("jj")
.arg(subcommand)
.args(path)
.arg("--help")
.stderr(Stdio::null())
.output()
.context("failed to spawn jj for preflight")?;
if !output.status.success() {
// unknown subcommand or other error — let the main flow surface jj's error
return Ok(true);
return Ok(None);
}
let help = String::from_utf8_lossy(&output.stdout);
Ok(help.contains("REVSET"))
Ok(Some(String::from_utf8_lossy(&output.stdout).into_owned()))
}
pub fn exec(subcommand: &str, passthrough: &[String], ids: &[String]) -> Result<ExitStatus> {
let revset = ids.join("|");
let status = Command::new("jj")
.arg(subcommand)
.args(passthrough)
.arg("-r")
.arg(&revset)
.status()
.context("failed to spawn jj")?;
/// Parse the `Commands:` block of a jj help output. Returns the set of
/// subcommand names AND their `[aliases: …]` annotations, so a user typing
/// `jjf tag s` (where `s` is the alias for `set`) walks correctly.
fn parse_commands_section(help: &str) -> std::collections::HashSet<String> {
let mut set = std::collections::HashSet::new();
let mut in_commands = false;
for line in help.lines() {
if line == "Commands:" {
in_commands = true;
continue;
}
if !in_commands {
continue;
}
if line.is_empty() {
break;
}
if !line.starts_with(' ') {
break;
}
let trimmed = line.trim_start();
let indent = line.len() - trimmed.len();
// Primary entries have exactly 2-space indent. Anything deeper is
// a description continuation we don't care about.
if indent != 2 {
continue;
}
if let Some(name) = trimmed.split_whitespace().next() {
set.insert(name.to_string());
}
scan_aliases(line, &mut set);
}
set
}
fn scan_aliases(line: &str, names: &mut std::collections::HashSet<String>) {
let Some(start) = line.find("[aliases:") else {
return;
};
let after = &line[start + "[aliases:".len()..];
let Some(end) = after.find(']') else {
return;
};
for a in after[..end].split(',') {
let n = a.trim();
if !n.is_empty() {
names.insert(n.to_string());
}
}
}
/// Pick the right revset-accepting flag from a leaf's help, preferring
/// `-r`/`--revision[s]` over `-t, --to`. Returns `None` if the leaf has
/// neither.
pub fn parse_revset_flag(help: &str) -> Option<RevsetFlag> {
let opts = parse_options(help);
for opt in &opts {
if opt.is_revset
&& opt
.longs
.iter()
.any(|n| n == "--revision" || n == "--revisions")
{
return Some(RevsetFlag::Revision);
}
}
for opt in &opts {
if opt.is_revset && opt.longs.iter().any(|n| n == "--to") {
return Some(RevsetFlag::To);
}
}
None
}
#[derive(Debug)]
struct OptionEntry {
longs: Vec<String>,
/// True when the placeholder is `<REVSET>` or `<REVSETS>`.
is_revset: bool,
}
fn parse_options(help: &str) -> Vec<OptionEntry> {
let mut opts = Vec::new();
let mut in_options = false;
let mut current: Option<OptionEntry> = None;
for line in help.lines() {
if line == "Options:" {
in_options = true;
continue;
}
if !in_options {
continue;
}
// A non-indented non-empty line ends `Options:` (e.g. `Global Options:`).
if !line.is_empty() && !line.starts_with(' ') {
if let Some(o) = current.take() {
opts.push(o);
}
break;
}
if line.is_empty() {
// Blank lines separate options inside the section; don't end it.
continue;
}
let trimmed = line.trim_start();
let indent = line.len() - trimmed.len();
// New entry: 2-space indent with `-X,` (short + long) OR 6-space
// indent with `--` (long-only flag, e.g. `--allow-move`).
let is_new_entry = (indent == 2 && trimmed.starts_with('-'))
|| (indent == 6 && trimmed.starts_with("--"));
if is_new_entry {
if let Some(o) = current.take() {
opts.push(o);
}
current = Some(parse_option_header(trimmed));
} else if let Some(ref mut o) = current {
// Continuation line. Pull any `[aliases: --foo, --bar]` into the
// current option's long-name set so the priority match sees them.
let mut alias_set = std::collections::HashSet::new();
scan_aliases(line, &mut alias_set);
for n in alias_set {
if n.starts_with("--") {
o.longs.push(n);
}
}
}
}
if let Some(o) = current {
opts.push(o);
}
opts
}
fn parse_option_header(s: &str) -> OptionEntry {
let mut longs = Vec::new();
let mut placeholder: Option<String> = None;
for token in s.split(|c: char| c == ',' || c.is_whitespace()) {
let t = token.trim();
if t.is_empty() {
continue;
}
if let Some(rest) = t.strip_prefix("--") {
// Strip a trailing `=...` if clap ever emits one (it doesn't
// today, but cheap insurance).
let name = rest.split('=').next().unwrap_or(rest);
longs.push(format!("--{name}"));
} else if let Some(stripped) = t.strip_prefix('<') {
// Placeholder: `<REVSET>`, `<REVSETS>`, `<NAMES>...`, etc.
let inner = stripped.trim_end_matches("...");
if let Some(end) = inner.find('>') {
placeholder = Some(inner[..end].to_string());
}
}
}
let is_revset = matches!(placeholder.as_deref(), Some("REVSET") | Some("REVSETS"));
OptionEntry { longs, is_revset }
}
pub fn exec(target: &RevsetTarget, ids: &[String]) -> Result<ExitStatus> {
let mut cmd = Command::new("jj");
cmd.args(&target.leaf);
cmd.args(&target.passthrough);
if !ids.is_empty() {
if let Some(flag) = target.flag {
cmd.arg(flag.as_str());
cmd.arg(ids.join("|"));
}
}
let status = cmd.status().context("failed to spawn jj")?;
Ok(status)
}
pub fn command_line(subcommand: &str, passthrough: &[String], ids: &[String]) -> String {
let mut parts: Vec<String> = Vec::with_capacity(4 + passthrough.len());
pub fn command_line(target: &RevsetTarget, ids: &[String]) -> String {
let mut parts: Vec<String> =
Vec::with_capacity(2 + target.leaf.len() + target.passthrough.len() + 2);
parts.push("jj".into());
parts.push(subcommand.into());
for a in passthrough {
for w in &target.leaf {
parts.push(w.clone());
}
for a in &target.passthrough {
parts.push(shell_quote(a));
}
if !ids.is_empty() {
parts.push("-r".into());
parts.push(shell_quote(&ids.join("|")));
if let Some(flag) = target.flag {
parts.push(flag.as_str().into());
parts.push(shell_quote(&ids.join("|")));
}
}
parts.join(" ")
}
@@ -326,3 +578,147 @@ pub fn shell_quote(s: &str) -> String {
out.push('\'');
out
}
#[cfg(test)]
mod tests {
use super::*;
const TAG_SET_HELP: &str = "Create or update tags
Usage: jj tag set [OPTIONS] <NAMES>...
Arguments:
<NAMES>...
Tag names to create or update
Options:
-r, --revision <REVSET>
Target revision to point to
[default: @]
[aliases: --to]
--allow-move
Allow moving existing tags
-h, --help
Print help (see a summary with '-h')
Global Options:
-R, --repository <REPOSITORY>
Path to repository
";
const BOOKMARK_MOVE_HELP: &str = "Move existing bookmarks to target revision
Usage: jj bookmark move [OPTIONS] <NAMES|--from <REVSETS>>
Arguments:
[NAMES]...
Move bookmarks matching the given name patterns
Options:
-f, --from <REVSETS>
Move bookmarks from the given revisions
-t, --to <REVSET>
Move bookmarks to this revision
[default: @]
-B, --allow-backwards
Allow moving bookmarks backwards or sideways
-h, --help
Print help (see a summary with '-h')
Global Options:
-R, --repository <REPOSITORY>
Path to repository
";
const TAG_PARENT_HELP: &str = "Manage tags
Usage: jj tag [OPTIONS] <COMMAND>
Commands:
delete Delete existing tags [aliases: d]
list List tags and their targets [aliases: l]
set Create or update tags [aliases: s]
Options:
-h, --help
Print help (see a summary with '-h')
Global Options:
-R, --repository <REPOSITORY>
Path to repository
";
#[test]
fn revset_flag_prefers_revision_over_to_alias() {
// tag set has `-r, --revision <REVSET>` with `[aliases: --to]`.
// The Revision-class match must win over the To-class match.
assert_eq!(parse_revset_flag(TAG_SET_HELP), Some(RevsetFlag::Revision));
}
#[test]
fn revset_flag_falls_back_to_to_when_no_revision() {
// bookmark move has only `-f, --from <REVSETS>` and `-t, --to <REVSET>`.
assert_eq!(parse_revset_flag(BOOKMARK_MOVE_HELP), Some(RevsetFlag::To));
}
#[test]
fn revset_flag_none_for_parent_without_options() {
assert_eq!(parse_revset_flag(TAG_PARENT_HELP), None);
}
#[test]
fn commands_section_includes_names_and_aliases() {
let cmds = parse_commands_section(TAG_PARENT_HELP);
for name in ["delete", "list", "set", "d", "l", "s"] {
assert!(cmds.contains(name), "missing {name} in {cmds:?}");
}
}
#[test]
fn commands_section_empty_for_leaf() {
let cmds = parse_commands_section(BOOKMARK_MOVE_HELP);
assert!(cmds.is_empty(), "expected no commands, got {cmds:?}");
}
#[test]
fn command_line_emits_revision_flag() {
let target = RevsetTarget {
leaf: vec!["tag".into(), "set".into()],
passthrough: vec!["v0.2.0".into()],
flag: Some(RevsetFlag::Revision),
};
let s = command_line(&target, &["abcd1234".into()]);
assert_eq!(s, "jj tag set v0.2.0 -r abcd1234");
}
#[test]
fn command_line_emits_to_flag() {
let target = RevsetTarget {
leaf: vec!["bm".into()],
passthrough: vec!["main".into()],
flag: Some(RevsetFlag::To),
};
let s = command_line(&target, &["abcd1234".into()]);
assert_eq!(s, "jj bm main --to abcd1234");
}
#[test]
fn command_line_omits_flag_when_no_ids() {
let target = RevsetTarget {
leaf: vec!["describe".into()],
passthrough: vec!["-r".into(), "@".into()],
flag: Some(RevsetFlag::Revision),
};
// When the user already supplied -r, jjf calls command_line with empty
// ids — no extra flag should be appended.
let s = command_line(&target, &[]);
assert_eq!(s, "jj describe -r @");
}
}
+48 -20
View File
@@ -25,19 +25,19 @@ fn real_main() -> Result<u8> {
match first.as_str() {
"init" => run_init(args),
"--emit" => match args.next() {
Some(sub) => {
let passthrough: Vec<String> = args.collect();
run_pick(&sub, &passthrough, /* emit = */ true)
}
None => {
"--emit" => {
let rest: Vec<String> = args.collect();
if rest.is_empty() {
eprintln!("usage: jjf --emit <jj-subcommand> [args...]");
Ok(2)
return Ok(2);
}
},
sub => {
let passthrough: Vec<String> = args.collect();
run_pick(sub, &passthrough, /* emit = */ false)
run_pick(&rest, /* emit = */ true)
}
_ => {
let mut all = Vec::with_capacity(1 + args.size_hint().0);
all.push(first);
all.extend(args);
run_pick(&all, /* emit = */ false)
}
}
}
@@ -65,10 +65,29 @@ fn run_init(mut args: impl Iterator<Item = String>) -> Result<u8> {
}
}
fn run_pick(subcommand: &str, passthrough: &[String], emit: bool) -> Result<u8> {
if !jj::supports_revisions(subcommand)? {
fn run_pick(args: &[String], emit: bool) -> Result<u8> {
let target = jj::resolve_target(args)?;
// Case 1: user already supplied a revset flag — bypass the picker entirely
// and run their command verbatim. Avoids double-`-r` and respects what
// they explicitly asked for.
if has_user_revset_flag(&target.passthrough) {
if emit {
let cmd = jj::command_line(&target, &[]);
println!("{cmd}");
return Ok(0);
}
echo_command(&target, &[]);
let status = jj::exec(&target, &[])?;
return Ok(status.code().unwrap_or(1) as u8);
}
// Case 2: leaf accepts no revset flag at all (e.g. `jj operation`,
// `jj workspace list`). Refuse to run the picker.
if target.flag.is_none() {
let leaf = target.leaf.join(" ");
eprintln!(
"jjf: 'jj {subcommand}' doesn't take revisions — run 'jj {subcommand}' directly."
"jjf: 'jj {leaf}' takes no revset flag (-r/--to) — run 'jj {leaf}' directly."
);
return Ok(2);
}
@@ -89,7 +108,7 @@ fn run_pick(subcommand: &str, passthrough: &[String], emit: bool) -> Result<u8>
None
};
let tui_result = tui::run(rows, subcommand, passthrough);
let tui_result = tui::run(rows, &target);
if let Some(saved) = saved_stdout {
restore_stdout(saved)?;
@@ -103,16 +122,25 @@ fn run_pick(subcommand: &str, passthrough: &[String], emit: bool) -> Result<u8>
}
if emit {
let cmd = jj::command_line(subcommand, passthrough, &ids);
let cmd = jj::command_line(&target, &ids);
println!("{cmd}");
Ok(0)
} else {
echo_command(subcommand, passthrough, &ids);
let status = jj::exec(subcommand, passthrough, &ids)?;
echo_command(&target, &ids);
let status = jj::exec(&target, &ids)?;
Ok(status.code().unwrap_or(1) as u8)
}
}
fn has_user_revset_flag(passthrough: &[String]) -> bool {
passthrough.iter().any(|a| {
matches!(
a.as_str(),
"-r" | "--revision" | "--revisions" | "-t" | "--to"
)
})
}
#[cfg(unix)]
fn redirect_stdout_to_tty() -> Result<libc::c_int> {
use std::os::fd::AsRawFd;
@@ -142,9 +170,9 @@ fn restore_stdout(saved: libc::c_int) -> Result<()> {
Ok(())
}
fn echo_command(subcommand: &str, passthrough: &[String], ids: &[String]) {
fn echo_command(target: &jj::RevsetTarget, ids: &[String]) {
use std::io::IsTerminal;
let cmd = jj::command_line(subcommand, passthrough, ids);
let cmd = jj::command_line(target, ids);
if std::io::stdout().is_terminal() {
println!("\x1b[2m$\x1b[0m {cmd}");
} else {
+6 -9
View File
@@ -278,8 +278,7 @@ fn score_row(
pub fn run(
rows: Vec<Row>,
subcommand: &str,
passthrough: &[String],
target: &jj::RevsetTarget,
) -> Result<Option<Vec<String>>> {
install_panic_hook();
let mut tty = OpenOptions::new()
@@ -295,7 +294,7 @@ pub fn run(
tty.write_all(HIDE_CURSOR).context("hide cursor")?;
tty.flush().ok();
let result = run_loop(&mut tty, rows, subcommand, passthrough);
let result = run_loop(&mut tty, rows, target);
let _ = tty.write_all(SHOW_CURSOR);
let _ = tty.write_all(b"\x1b[?1049l");
@@ -320,15 +319,14 @@ fn install_panic_hook() {
fn run_loop(
tty: &mut File,
rows: Vec<Row>,
subcommand: &str,
passthrough: &[String],
target: &jj::RevsetTarget,
) -> Result<Option<Vec<String>>> {
let mut app = App::new(rows);
let (mut term_cols, mut term_rows) = crossterm::terminal::size().unwrap_or((80, 24));
// Chrome rows: input(1) + cmd_preview(1) + hint(1) = 3.
app.last_height = (term_rows as usize).saturating_sub(3);
loop {
render(tty, &mut app, term_rows, term_cols, subcommand, passthrough)?;
render(tty, &mut app, term_rows, term_cols, target)?;
if !event::poll(Duration::from_millis(250)).context("event poll failed")? {
continue;
}
@@ -454,8 +452,7 @@ fn render(
app: &mut App,
term_rows: u16,
term_cols: u16,
subcommand: &str,
passthrough: &[String],
target: &jj::RevsetTarget,
) -> Result<()> {
let viewport_y: u16 = 0;
let viewport_height = term_rows;
@@ -601,7 +598,7 @@ fn render(
let cmd_y = viewport_y + 1 + log_height as u16 + 1;
write!(buf, "\x1b[{};1H", cmd_y)?;
buf.extend_from_slice(ERASE_TO_EOL);
let cmd = jj::command_line(subcommand, passthrough, &app.pending_ids());
let cmd = jj::command_line(target, &app.pending_ids());
let cmd_max = (term_cols as usize).saturating_sub(2); // for "▶ "
let cmd_visible: String = if cmd.chars().count() <= cmd_max {
cmd