Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/spec/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,44 @@ The priority over which is used (CLI flag, env var, config file, default) is the
are defined,
so in this example it will be "CLI flag > env var > config file > default".

## Command effects

A command can declare what running it does to the world:

```kdl
cmd "ls" effect="read" help="List installed tools"
cmd "use" effect="write" help="Install a tool and add it to the config"
cmd "uninstall" effect="destructive" help="Remove a tool"
```

| Effect | Meaning |
| -------------- | ------------------------------------------------------------------------------------ |
| `read` | Only inspects state. Running it twice is the same as running it once. |
| `write` | Creates or modifies state, but removes nothing the user cannot recreate. |
| `destructive` | May delete or irreversibly overwrite something. Deserves a confirmation prompt. |

This is a coarse classification, not a permission model. It exists because
several consumers keep reinventing the same distinction:

- generated documentation and `--help` can mark destructive commands
- a wrapper script can require confirmation before running one
- an AI coding agent can be handed an allowlist of read-only commands instead of
prompting on every invocation

`effect` is **not inherited by subcommands**. `git remote` and
`git remote remove` do different things, and quietly inheriting a parent's
effect would make the least safe reading of a spec the default one. A command
with no `effect` is unknown, not safe — consumers should treat the absence of a
value as "ask".

It can also be written as a child node, which is easier to generate:

```kdl
cmd "uninstall" {
effect "destructive"
}
```

## Compatibility

Usage is not designed to model every possible CLI. It's generally designed for CLIs that follow
Expand Down
7 changes: 7 additions & 0 deletions lib/src/docs/markdown/.cmd.rs.pending-snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{"run_id":"1785010937-360323306","line":141,"new":{"module_name":"usage__docs__markdown__cmd__tests","snapshot_name":"render_markdown_cmd_effect","metadata":{"source":"lib/src/docs/markdown/cmd.rs","assertion_line":141,"expression":"rendered"},"snapshot":"# `mise ls`\n\n- **Usage**: `mise ls`\n- **Effect**: read-only\n\nList installed tools\n# `mise use`\n\n- **Usage**: `mise use`\n- **Effect**: modifies state\n\nInstall a tool\n# `mise uninstall`\n\n- **Usage**: `mise uninstall`\n- **Effect**: destructive — may delete or irreversibly overwrite\n\nRemove a tool\n# `mise version`\n\n- **Usage**: `mise version`\n\nShow the version"},"old":{"module_name":"usage__docs__markdown__cmd__tests","metadata":{},"snapshot":"# `mise ls`\n\n- **Usage**: `mise ls`\n- **Effect**: read-only\n\nList installed tools\n\n# `mise use`\n\n- **Usage**: `mise use`\n- **Effect**: modifies state\n\nInstall a tool\n\n# `mise uninstall`\n\n- **Usage**: `mise uninstall`\n- **Effect**: destructive — may delete or irreversibly overwrite\n\nRemove a tool\n\n# `mise version`\n\n- **Usage**: `mise version`\n\nShow the version"}}
{"run_id":"1785010954-370828992","line":141,"new":null,"old":null}
{"run_id":"1785010954-370828992","line":27,"new":null,"old":null}
{"run_id":"1785010965-151987287","line":141,"new":null,"old":null}
{"run_id":"1785010965-151987287","line":27,"new":null,"old":null}
{"run_id":"1785011019-265767551","line":141,"new":null,"old":null}
{"run_id":"1785011019-265767551","line":27,"new":null,"old":null}
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
54 changes: 54 additions & 0 deletions lib/src/docs/markdown/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ impl MarkdownRenderer {
mod tests {
use crate::docs::markdown::renderer::MarkdownRenderer;
use crate::test::SPEC_KITCHEN_SINK;
use crate::Spec;
use insta::assert_snapshot;

#[test]
Expand Down Expand Up @@ -113,4 +114,57 @@ mod tests {
- [`mycli plugin <SUBCOMMAND>`](/plugin.md)
");
}

#[test]
fn test_render_markdown_cmd_effect() {
let spec: Spec = r#"
name "mise"
bin "mise"
cmd "ls" effect="read" help="List installed tools"
cmd "use" effect="write" help="Install a tool"
cmd "uninstall" effect="destructive" help="Remove a tool"
cmd "version" help="Show the version"
"#
.parse()
.unwrap();
let ctx = MarkdownRenderer::new(spec.clone()).with_multi(true);
let rendered = spec
.cmd
.subcommands
.values()
.map(|cmd| ctx.render_cmd(cmd).unwrap())
.collect::<Vec<_>>()
.join("\n\n");

// Every effect value must render its own label, and a command without
// one must not render the line at all.
assert_snapshot!(rendered, @r"
# `mise ls`

- **Usage**: `mise ls`
- **Effect**: read-only

List installed tools

# `mise use`

- **Usage**: `mise use`
- **Effect**: modifies state

Install a tool

# `mise uninstall`

- **Usage**: `mise uninstall`
- **Effect**: destructive — may delete or irreversibly overwrite

Remove a tool

# `mise version`

- **Usage**: `mise version`

Show the version
");
}
}
3 changes: 3 additions & 0 deletions lib/src/docs/markdown/templates/cmd_template.md.tera
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
{%- if cmd.aliases %}
- **Aliases**: `{{ cmd.aliases | join(sep="`, `") }}`
{%- endif %}
{%- if cmd.effect %}
- **Effect**: {% if cmd.effect == "read" %}read-only{% elif cmd.effect == "destructive" %}destructive — may delete or irreversibly overwrite{% else %}modifies state{% endif %}
{%- endif %}
Comment on lines +15 to +17

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Effect rendering lacks snapshot coverage

The new template branches for read, write, and destructive are not exercised by the existing Markdown snapshots because the shared test spec declares no effects. Add cases for all three values so changes to enum serialization or template comparisons cannot silently produce incorrect labels.

Knowledge Base Used: Docs generation: rendering a Spec into markdown, manpages, and CLI help

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

{%- if source_code_link %}
- **Source code**: {{ source_code_link }}
{%- endif %}
Expand Down
3 changes: 3 additions & 0 deletions lib/src/docs/models.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::docs::markdown::MarkdownRenderer;
use crate::spec::effect::SpecCommandEffect;
use crate::SpecChoices;
use indexmap::IndexMap;
use serde::Serialize;
Expand Down Expand Up @@ -37,6 +38,7 @@ pub struct SpecCommand {
pub flags: Vec<SpecFlag>,
// pub mounts: Vec<SpecMount>,
pub deprecated: Option<String>,
pub effect: Option<SpecCommandEffect>,
pub hide: bool,
pub subcommand_required: bool,
pub help: Option<String>,
Expand Down Expand Up @@ -214,6 +216,7 @@ impl From<&crate::SpecCommand> for SpecCommand {
flags,
// mounts: cmd.mounts.iter().map(SpecMount::from).collect(),
deprecated: cmd.deprecated.clone(),
effect: cmd.effect,
hide: cmd.hide,
subcommand_required: cmd.subcommand_required,
help: cmd.help.clone(),
Expand Down
1 change: 1 addition & 0 deletions lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub use crate::spec::builder::{SpecArgBuilder, SpecCommandBuilder, SpecFlagBuild
pub use crate::spec::choices::SpecChoices;
pub use crate::spec::cmd::SpecCommand;
pub use crate::spec::complete::SpecComplete;
pub use crate::spec::effect::SpecCommandEffect;
pub use crate::spec::flag::SpecFlag;
pub use crate::spec::mount::SpecMount;
pub use crate::spec::Spec;
Expand Down
7 changes: 7 additions & 0 deletions lib/src/spec/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
//! ```

use crate::spec::cmd::SpecExample;
use crate::spec::effect::SpecCommandEffect;
use crate::{spec::arg::SpecDoubleDashChoices, SpecArg, SpecChoices, SpecCommand, SpecFlag};

/// Builder for SpecFlag
Expand Down Expand Up @@ -464,6 +465,12 @@ impl SpecCommandBuilder {
self
}

/// Set what running this command does to the world
pub fn effect(mut self, effect: SpecCommandEffect) -> Self {
self.inner.effect = Some(effect);
self
}

/// Set deprecated message
pub fn deprecated(mut self, msg: impl Into<String>) -> Self {
self.inner.deprecated = Some(msg.into());
Expand Down
155 changes: 155 additions & 0 deletions lib/src/spec/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::error::UsageErr;
use crate::sh::sh;
use crate::spec::builder::SpecCommandBuilder;
use crate::spec::context::ParsingContext;
use crate::spec::effect::{SpecCommandEffect, EFFECT_VALUES};
use crate::spec::helpers::{string_entry, NodeHelper};
use crate::spec::is_false;
use crate::spec::mount::SpecMount;
Expand Down Expand Up @@ -49,6 +50,10 @@ pub struct SpecCommand {
/// Deprecation message if this command is deprecated
#[serde(skip_serializing_if = "Option::is_none")]
pub deprecated: Option<String>,
/// What running this command does to the world: read, write or destructive.
/// Not inherited by subcommands.
#[serde(skip_serializing_if = "Option::is_none")]
pub effect: Option<SpecCommandEffect>,
Comment thread
greptile-apps[bot] marked this conversation as resolved.
/// Whether to hide this command from help output
pub hide: bool,
/// True when this command came from a [`SpecMount`], i.e. it describes another
Expand Down Expand Up @@ -131,6 +136,7 @@ impl Default for SpecCommand {
flags: vec![],
mounts: vec![],
deprecated: None,
effect: None,
hide: false,
mounted: false,
flags_from_mount: false,
Expand Down Expand Up @@ -221,6 +227,17 @@ impl SpecCommand {
"after_help_md" => cmd.after_help_md = Some(v.ensure_string()?),
"subcommand_required" => cmd.subcommand_required = v.ensure_bool()?,
"hide" => cmd.hide = v.ensure_bool()?,
"effect" => {
let raw = v.ensure_string()?;
match raw.parse() {
Ok(effect) => cmd.effect = Some(effect),
Err(_) => bail_parse!(
ctx,
v.entry.span(),
"unsupported effect {raw}, expected one of: {EFFECT_VALUES}"
),
}
}
"restart_token" => cmd.restart_token = Some(v.ensure_string()?),
"deprecated" => {
cmd.deprecated = match v.value.as_bool() {
Expand Down Expand Up @@ -294,6 +311,18 @@ impl SpecCommand {
cmd.subcommand_required = child.ensure_arg_len(1..=1)?.arg(0)?.ensure_bool()?
}
"hide" => cmd.hide = child.ensure_arg_len(1..=1)?.arg(0)?.ensure_bool()?,
"effect" => {
let arg = child.ensure_arg_len(1..=1)?.arg(0)?;
let raw = arg.ensure_string()?;
match raw.parse() {
Ok(effect) => cmd.effect = Some(effect),
Err(_) => bail_parse!(
ctx,
arg.entry.span(),
"unsupported effect {raw}, expected one of: {EFFECT_VALUES}"
),
}
}
"restart_token" => {
cmd.restart_token = Some(child.ensure_arg_len(1..=1)?.arg(0)?.ensure_string()?)
}
Expand Down Expand Up @@ -412,6 +441,9 @@ impl SpecCommand {
}
self.hide = other.hide;
self.subcommand_required = other.subcommand_required;
if other.effect.is_some() {
self.effect = other.effect;
}
if other.restart_token.is_some() {
self.restart_token = other.restart_token;
}
Expand Down Expand Up @@ -574,6 +606,10 @@ impl From<&SpecCommand> for KdlNode {
node.entries_mut()
.push(string_entry(Some("deprecated"), deprecated));
}
if let Some(effect) = &cmd.effect {
node.entries_mut()
.push(string_entry(Some("effect"), effect.as_str()));
}
for flag in &cmd.flags {
let children = node.children_mut().get_or_insert_with(KdlDocument::new);
children.nodes_mut().push(flag.into());
Expand Down Expand Up @@ -644,3 +680,122 @@ impl From<clap::Command> for Spec {
(&cmd).into()
}
}

#[cfg(test)]
mod tests {
use crate::spec::effect::SpecCommandEffect;
use crate::Spec;
use insta::assert_snapshot;

#[test]
fn test_effect_prop_and_child_node() {
let spec = Spec::parse(
&Default::default(),
r#"
bin "mise"
cmd "ls" effect="read"
cmd "use" effect="write"
cmd "uninstall" {
effect "destructive"
}
cmd "version"
"#,
)
.unwrap();

let cmds = &spec.cmd.subcommands;
assert_eq!(cmds["ls"].effect, Some(SpecCommandEffect::Read));
assert_eq!(cmds["use"].effect, Some(SpecCommandEffect::Write));
assert_eq!(
cmds["uninstall"].effect,
Some(SpecCommandEffect::Destructive)
);
// Unspecified stays unknown rather than defaulting to anything.
assert_eq!(cmds["version"].effect, None);
}

#[test]
fn test_effect_is_not_inherited_by_subcommands() {
let spec = Spec::parse(
&Default::default(),
r#"
bin "git"
cmd "remote" effect="read" {
cmd "add" effect="write"
cmd "show"
}
"#,
)
.unwrap();

let remote = &spec.cmd.subcommands["remote"];
assert_eq!(remote.effect, Some(SpecCommandEffect::Read));
assert_eq!(
remote.subcommands["add"].effect,
Some(SpecCommandEffect::Write)
);
assert_eq!(remote.subcommands["show"].effect, None);
}

#[test]
fn test_effect_roundtrips_through_kdl() {
let spec = Spec::parse(
&Default::default(),
r#"
bin "mise"
cmd "ls" effect="read"
cmd "uninstall" effect="destructive"
"#,
)
.unwrap();

assert_snapshot!(spec, @r#"
name mise
bin mise
cmd ls effect=read
cmd uninstall effect=destructive
"#);
}

/// `merge` is how included and mounted specs are composed onto a command.
/// It has to treat `effect` the way it treats every other optional field:
/// an overlay that says nothing must not erase what is already declared.
#[test]
fn test_effect_survives_merge() {
let cmd_with = |src: &str| {
Spec::parse(&Default::default(), src)
.unwrap()
.cmd
.subcommands["uninstall"]
.clone()
};

let declared = cmd_with(r#"cmd "uninstall" effect="destructive""#);
let silent = cmd_with(r#"cmd "uninstall" help="Remove a tool""#);
let contradicting = cmd_with(r#"cmd "uninstall" effect="write""#);

let mut cmd = declared.clone();
cmd.merge(silent);
assert_eq!(cmd.effect, Some(SpecCommandEffect::Destructive));

let mut cmd = declared;
cmd.merge(contradicting);
assert_eq!(cmd.effect, Some(SpecCommandEffect::Write));
}

#[test]
fn test_unknown_effect_is_an_error() {
let err = Spec::parse(
&Default::default(),
r#"
bin "mise"
cmd "ls" effect="readonly"
"#,
)
.unwrap_err();
assert!(
err.to_string().contains("Invalid usage config"),
"unexpected error: {err}"
);
}
}
Loading
Loading