diff --git a/CHANGELOG.md b/CHANGELOG.md index b1063b6..0e6f3e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,42 @@ that may never merge. They are not releases and are not listed here. API for the per-browser, per-country and per-host breakdown, which it now sends only on request. Without the flag the request is unchanged, and each product's `dimensions` in `-o json` is `{}`. +- `static get-image`, `static get-tile` and `tilesets get-tile` no longer + need their trailing optional positionals: ``, `` and + `` can be left off instead of passed as `""`. Writing `""` still + works, so existing scripts are unaffected. `--schema` now reports these + arguments as not required. +- A flag for a true/false API parameter now takes `=false`: + `--attribution=false` sends `attribution=false`. Alone it still sends + `true`, as before. Where the API defaults to `true` (`--attribution`, + `--logo`, `--autocomplete`, `--dedupe`, `--continue-straight` on driving + profiles), this is the only way to turn the setting off. A workflow + step's `false` still leaves the flag out, as before. +- `directions --snapping-include-closures` and + `--snapping-include-static-closures` take a value: `true` or `false` per + coordinate, semicolon-separated (`true;false`). They were switches that + sent a single `true`, which the API rejects with a 422 for any route, so + no working command changes. In `--schema` they change from a boolean + flag to a string option; this is a fix rather than a breaking change for + the same reason. +- `--help` shows each parameter's whole description rather than its first + sentence, so the allowed values and what else a parameter needs + ("Requires `overview=full`") are no longer cut off. +- An API command's `--help` shows the opening of its description as plain + text, followed by a `Docs:` link, instead of the whole Markdown page with + its headings, tables and curl examples (`geocoder batch` was 113 lines). + `--schema` still carries the full description. +- `--id` on a command that is not an API command (`history show`, + `auth profiles`, `config list` and the like) is now a usage error, exit + code 2. It used to be ignored: `history show --id x` printed the newest + run as if it were `x`. +- `--schema`: `env` and `default` are filled for every argument, including + the ones read by hand (`--timeout`, `--output`, `--no-color`, `--profile`, + `workflow install --repo`/`--ref`), which only mentioned them in their + description. A new top-level `groups` list carries what a command group's + own help says, such as `mapbox history`, which `commands` (leaves only) + never included. +- `feedback` moved from the Search help group to Maps and data. ## 1.1.1 - 2026-10-22 diff --git a/Cargo.lock b/Cargo.lock index c19a4a9..cf52f9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -828,6 +828,7 @@ dependencies = [ "jsonc-parser", "nix", "open", + "pulldown-cmark", "rand", "reqwest", "serde", @@ -975,6 +976,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "pulldown-cmark" +version = "0.13.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "278e9268c93b9e498a2da46c34d90ba93a3255eb99009646398f34bc73ab36eb" +dependencies = [ + "bitflags", + "memchr", + "unicase", +] + [[package]] name = "quinn" version = "0.11.12" diff --git a/Cargo.toml b/Cargo.toml index 0654710..7592fc1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -119,6 +119,12 @@ terminal-colorsaurus = "1" # a signal handler takes `unsafe` this crate does not have. Installed by # that command only; every other command keeps the default. ctrlc = { version = "3", features = ["termination"] } +# Reads the specs' Markdown descriptions for `--help`: the opening prose +# only, with its inline markup rendered rather than printed. A parser rather +# than pattern matching, because stripping markers by hand is what turned +# the code spans `and` and `x` into "and or x". Default features off: they +# are the HTML renderer and a command-line binary. +pulldown-cmark = { version = "0.13", default-features = false } [target.'cfg(unix)'.dependencies] # For `workflow run`: passing a SIGTERM on to the running step, which a diff --git a/custom-openapi/directions/openapi/directions.yaml b/custom-openapi/directions/openapi/directions.yaml index 1a1af28..7392c26 100644 --- a/custom-openapi/directions/openapi/directions.yaml +++ b/custom-openapi/directions/openapi/directions.yaml @@ -58,8 +58,7 @@ paths: live traffic), `mapbox/driving`, `mapbox/walking`, or `mapbox/cycling` are documented, but not necessarily exhaustive: some accounts have additional profiles of their - own. Sent exactly as typed; the API is the authority on - whether a value is valid, not this description. + own. Any other value is sent as typed. schema: type: string minLength: 1 @@ -251,8 +250,8 @@ paths: required: false description: >- Zero-based indices into `coordinates` marking which ones get - their own arrival instruction — must include `0` and the last - index. Requires `steps=true`. + their own arrival instruction, semicolon-separated — must + include `0` and the last index. Requires `steps=true`. schema: type: string example: "0,2" @@ -292,12 +291,6 @@ paths: - name: "alley_bias" in: query required: false - # Content before the profile note, not after: `first_sentence` in - # `src/main.rs` cuts a `--help` line at the first `.`, and - # `` `mapbox/driving` only. `` — a real sentence on its own — was - # eating everything that followed it. `--schema` and - # `docs/commands.md` show the description in full either way — - # this ordering is only for the one-line case. description: >- Bias the route against (negative) or toward (positive) alleys, -1 to 1. `mapbox/driving` only. @@ -352,22 +345,29 @@ paths: type: number minimum: 0 maximum: 100 + # One value per coordinate, not one switch: a lone `true` with two + # coordinates is a 422 ("one entry per coordinate, separated by + # ';'"), verified against production on 2026-10-23. - name: "snapping_include_closures" in: query required: false description: >- - Allow snapping to roads carrying a live-traffic closure. - `mapbox/driving-traffic` only. + Whether each coordinate may snap to a road carrying a live-traffic closure — + `true` or `false` per coordinate, semicolon-separated, with an + empty entry to skip one (`true;;false`). `mapbox/driving-traffic` + only. schema: - type: boolean + type: string - name: "snapping_include_static_closures" in: query required: false description: >- - Allow snapping to roads carrying a long-term closure. - `mapbox/driving-traffic` only. + Whether each coordinate may snap to a road carrying a long-term closure — + `true` or `false` per coordinate, semicolon-separated, with an + empty entry to skip one (`true;;false`). `mapbox/driving-traffic` + only. schema: - type: boolean + type: string - name: "walking_speed" in: query required: false diff --git a/custom-openapi/feedback/openapi/feedback.yaml b/custom-openapi/feedback/openapi/feedback.yaml index 4c1b6de..e6fe9d1 100644 --- a/custom-openapi/feedback/openapi/feedback.yaml +++ b/custom-openapi/feedback/openapi/feedback.yaml @@ -26,8 +26,8 @@ paths: Every feedback item on the account, oldest received first by default — filterable by id, status, category, a free-text search phrase, trace id, or a time window on when it was created, - received, or last updated. Paginated with `--after`/`start_cursor`/ - `end_cursor` the way every other listing on this CLI is. + received, or last updated. Paginated: pass a response's + `end_cursor` to `--after` for the next page. parameters: - name: "access_token" in: query @@ -111,10 +111,6 @@ paths: the feedback. schema: type: string - # "ISO 8601" trails each of these six rather than leads them: - # `first_sentence` in `src/main.rs` cuts a `--help` line at the - # first `.`, and "ISO 8601." on its own left `--help` showing just - # that. `--schema` and `docs/commands.md` still show each in full. - name: "created_before" in: query required: false @@ -207,11 +203,19 @@ paths: description: >- The feedback category. Not checked against a list; `feedback list` shows the ones already in use. + # The API accepts any coordinates here and never returns the + # feature, so the order is GeoJSON's by definition rather than + # by test: on 2026-10-23 both [95, 24.94] and [24.94, 95] were + # accepted. docs.mapbox.com says [latitude, longitude]. feature: type: object description: >- A GeoJSON Feature the feedback is about, with coordinates in GeoJSON's [longitude, latitude] order. + # Verified against production on 2026-10-23: bare base64 is a + # 400 ("Image must be a base64 data URI of one of: image/png, + # image/jpeg, image/jpg"), a data URI is accepted. + # docs.mapbox.com says plain base64, which is wrong. screenshot: type: string description: >- diff --git a/custom-openapi/isochrone/openapi/isochrone.yaml b/custom-openapi/isochrone/openapi/isochrone.yaml index 9aff3fa..dfc8a4e 100644 --- a/custom-openapi/isochrone/openapi/isochrone.yaml +++ b/custom-openapi/isochrone/openapi/isochrone.yaml @@ -47,8 +47,7 @@ paths: live traffic), `mapbox/driving`, `mapbox/walking`, or `mapbox/cycling` are documented, but not necessarily exhaustive: some accounts have additional profiles of their - own. Sent exactly as typed; the API is the authority on - whether a value is valid, not this description. + own. Any other value is sent as typed. schema: type: string minLength: 1 @@ -113,14 +112,6 @@ paths: - name: "denoise" in: query required: false - # Spelled "0 to 1"/"1", not "0.0-1.0"/"1.0": `first_sentence` in - # `src/main.rs` cuts a `--help` line at the first `.`, and this - # value's own bounds are the first digits that would appear no - # matter where in the sentence they sit, so a literal decimal - # point anywhere before the end truncates it — not just at the - # front, the way `directions.yaml`'s own fix assumed. `--schema` - # and `docs/commands.md` still show the fuller description below - # in full, decimals included. description: >- A larger value removes more of the smaller contours, 0 to 1, defaulting to 1. A value of 1 returns only the largest contour diff --git a/custom-openapi/map-match/openapi/map-match.yaml b/custom-openapi/map-match/openapi/map-match.yaml index 0e0ad0a..058434d 100644 --- a/custom-openapi/map-match/openapi/map-match.yaml +++ b/custom-openapi/map-match/openapi/map-match.yaml @@ -49,8 +49,7 @@ paths: live traffic), `mapbox/driving`, `mapbox/walking`, or `mapbox/cycling` are documented, but not necessarily exhaustive: some accounts have additional profiles of their - own. Sent exactly as typed; the API is the authority on - whether a value is valid, not this description. + own. Any other value is sent as typed. schema: type: string minLength: 1 @@ -95,9 +94,10 @@ paths: in: query required: false description: >- - Which side of the road to approach each waypoint from, - semicolon-separated — `unrestricted` or `curb` per coordinate. - Requires `--steps`. + Which side of the road to approach each waypoint from, one per + waypoint (every coordinate unless `--waypoints` picks some), + semicolon-separated — `unrestricted` or `curb`. Requires + `--steps`. schema: type: string - name: "geometries" diff --git a/custom-openapi/matrix/openapi/matrix.yaml b/custom-openapi/matrix/openapi/matrix.yaml index f7b9edf..7304172 100644 --- a/custom-openapi/matrix/openapi/matrix.yaml +++ b/custom-openapi/matrix/openapi/matrix.yaml @@ -46,13 +46,12 @@ paths: # named beyond `driving`/`walking`/`cycling`. Same fix as # `directions.yaml`'s `profile`. description: >- - The routing profile. `mapbox/driving-traffic` (accounts for - live traffic, caps at 10 coordinates instead of 25), - `mapbox/driving`, `mapbox/walking`, and `mapbox/cycling` are + The routing profile — `mapbox/driving-traffic` (accounts for + live traffic, 10 coordinates at most instead of 25), + `mapbox/driving`, `mapbox/walking`, or `mapbox/cycling` are documented, but not necessarily exhaustive: some accounts have - additional profiles of their own. Sent exactly as typed, the - API is the authority on whether a value is valid, not this - description. + additional profiles of their own. Any other value is sent as + typed. schema: type: string minLength: 1 @@ -100,17 +99,18 @@ paths: filtering the road segments considered by direction of travel. schema: type: string + # Semicolons, never commas: verified against production, a + # comma-separated list is a 422 ("may be \"all\" or + # semicolon-separated list of 0-based integer indices"). Other index + # lists on the Navigation commands vary per parameter, so this one + # says so explicitly. - name: "sources" in: query required: false description: >- - Which coordinates act as sources (matrix rows) — `all` - (the default), or zero-based indices, semicolon-separated. - Verified against production: a comma-separated list is a 422, - "may be \"all\" or semicolon-separated list of 0-based integer - indices" — unlike every other index/value list on this CLI's - Navigation commands, which are comma- or semicolon-separated - per parameter but never comma where this API wants semicolons. + Which coordinates act as sources (matrix rows) — `all` (the + default), or zero-based indices separated by semicolons + (`0;2`), not commas. schema: type: string example: "0;2" @@ -119,18 +119,14 @@ paths: required: false description: >- Which coordinates act as destinations (matrix columns) — `all` - (the default), or zero-based indices, semicolon-separated. See - `sources` above — comma-separated is a 422 here specifically. + (the default), or zero-based indices separated by semicolons + (`0;2`), not commas. schema: type: string example: "1;3" - name: "fallback_speed" in: query required: false - # Content before "Legacy", not after: `first_sentence` in - # `src/main.rs` cuts a `--help` line at the first `.`, and - # "Legacy." on its own left `--help` showing just that word. - # `--schema` and `docs/commands.md` still show it in full. description: >- Replaces a `null` (unreachable) cell with a straight-line estimate at this speed, km/h, rather than leaving it `null`. diff --git a/custom-openapi/places/openapi/places.yaml b/custom-openapi/places/openapi/places.yaml index 52f00e3..b89c151 100644 --- a/custom-openapi/places/openapi/places.yaml +++ b/custom-openapi/places/openapi/places.yaml @@ -36,12 +36,15 @@ paths: post: operationId: batch summary: Full detail for one or more places. + # `places` has no subcommand, so this, not `info.description`, is + # what `mapbox places --help` shows. It always sends the batch + # request, one id or many, so there is no get/batch choice to offer. description: >- - Each `mapbox_id` comes from a Search Box API result (`search - forward`/`reverse`/`category`) — this API does not look places up - by name or location itself. Always sends a batch request, one id - or many, so a single id reads the same way, no `get`/`batch` - choice to make. Up to 100 ids in one call. + Full detail for one or more places — hours, phone, website, + photos, address, coordinates — by the `mapbox_id`s a Search Box + result returned (`search forward`, `reverse` or `category`). Up to + 100 ids per call. Public Preview, with a 1000-records-per-account + monthly quota. parameters: - name: "access_token" in: query diff --git a/docs/commands.md b/docs/commands.md index f15ab97..639b21e 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -566,7 +566,7 @@ either. | `--use-login` | Ignore `MAPBOX_ACCESS_TOKEN`; use stored credentials. | | `--profile ` | Which stored credentials to use. | | `--output`, `-o` | `auto` \| `text` \| `json`. | -| `--id ` | On a command that returns a list, print just the row with that `id` or `name`. | +| `--id ` | On an API command that returns a list, print just the row with that `id` or `name`. Any other command rejects it. | | `--quiet`, `-q` | Don't print the `mapbox · v` banner, which goes to stderr and only when stderr is a terminal, the note after a download, or a workflow step's details. Also `MAPBOX_QUIET`. | | `--no-color` | Turn off color in help, errors and text output. Same as `NO_COLOR`; wins over `FORCE_COLOR`. | | `--timeout ` | How long one request may take, connection included. Defaults to 60 seconds, or 900 for a body read from `--file` or from a `--data @`/`@-`. Also `MAPBOX_TIMEOUT`. | @@ -608,6 +608,13 @@ Three things worth knowing about the read forms: `@` rather than `@-` to keep the prompt, or pass `--yes` to say the answer deliberately. +A flag that stands for a true/false API parameter, such as `--steps` or +`--attribution`, sends `true` when given alone. Write `--flag=false` to send +`false`, which matters where the API defaults to `true` (`--attribution`, +`--logo`, `--autocomplete`, `--dedupe`, `--continue-straight` on driving +profiles). The `=` is required: `--flag false` leaves `false` as a separate +argument. + A command that changes something takes `--dry-run`, which prints the request it would send, on stdout, and sends nothing. Which commands those are is not a list anyone keeps: it is every `POST`, `PUT`, `PATCH` and `DELETE` — 15 of @@ -962,7 +969,7 @@ is 2-25 `{longitude},{latitude}` pairs, semicolon-separated. | `--avoid-maneuver-radius <1-1000>` | Meters around the start to avoid a significant maneuver within. | | `--bearings ` | Filter road segments by direction of travel, one entry per coordinate. | | `--layers ` | A road layer (Z-order) per waypoint, for multi-level roads. | -| `--continue-straight` | Keep going straight at an intermediate waypoint rather than u-turning back to it. | +| `--continue-straight` | Keep going straight at an intermediate waypoint rather than u-turning back to it. Already the default on `mapbox/driving` and `mapbox/driving-traffic`; `--continue-straight=false` turns it off there. | | `--exclude ` | Road types or `point(lon lat)` values to route around, comma-separated (`motorway`, `toll`, `ferry`, `unpaved`, `cash_only_tolls`, `country_border`, `state_border`, `tunnel`). | | `--geometries ` | Route geometry format. Defaults to `polyline`. | | `--include ` | Special road types to allow, comma-separated (`hov2`, `hov3`, `hot`). | @@ -984,7 +991,7 @@ is 2-25 `{longitude},{latitude}` pairs, semicolon-separated. | `--arrive-by ` | `mapbox/driving` only. | | `--depart-at ` | `mapbox/driving` and `mapbox/driving-traffic`. | | `--max-height <0-10>` / `--max-width <0-10>` / `--max-weight <0-100>` | `mapbox/driving` and `mapbox/driving-traffic`. Meters, meters, metric tons. | -| `--snapping-include-closures` / `--snapping-include-static-closures` | `mapbox/driving-traffic` only. | +| `--snapping-include-closures ` / `--snapping-include-static-closures ` | `true` or `false` per coordinate, semicolon-separated; an empty entry skips one. `mapbox/driving-traffic` only. | | `--walking-speed <0.14-6.94>` / `--walkway-bias <-1..1>` | `mapbox/walking` only. | #### Examples @@ -1432,7 +1439,7 @@ geographic context and coordinates. | `--proximity ` | Bias results towards a point. | | `--bbox ` | Restrict to a box. | | `--language ` | IETF language tag. | -| `--autocomplete` | Partial-input matching. | +| `--autocomplete` | Partial-input matching. On by default; `--autocomplete=false` turns it off. | | `--worldview ` | Which country's view of disputed borders. | | `--permanent` | The result may be stored. Billed differently. | | `--format ` | `geojson` (default), or `v5` for the older response shape. | @@ -2697,9 +2704,11 @@ Tiles merged into this one command group (#116); `get-image` and `get-tile` are what were `static-images get-static-image` and `static-tiles get-static-tile`. -`` takes a **leading dot** — `.png`, `.jpeg`, `.webp`, or `""` for -the style default. `` is `@2x` or `""`. `` (`get-image` -only) is a marker/path/GeoJSON expression, or `""` for none. +`` takes a **leading dot** — `.png`, `.jpeg`, `.webp`, or left off +for the style default. `` is `@2x`, or left off. Both come last, so +either can be left off the end of the line; `""` still works in their place. +`` (`get-image` only) is a marker/path/GeoJSON expression, or `""` +for none. `""` for the overlay drops the segment entirely rather than sending an empty one, so a plain map image needs no overlay expression. @@ -2710,15 +2719,15 @@ A map image centered on a point. #### Parameters -Eleven positionals, in order: ` - `. +Nine positionals, then two optional ones, in order: ` + [highRes] [format]`. `` and `` default to `0` at the API, but the CLI requires -both as integers — pass `0 0` rather than `"" ""`. +both as numbers — pass `0 0` rather than `"" ""`. | Parameter | Effect | | --- | --- | -| `--attribution` / `--logo` | Keep or drop the Mapbox attribution and logo. | +| `--attribution` / `--logo` | Both on by default; `--attribution=false` / `--logo=false` drop them. | | `--addlayer ` | Add one layer on top of the style. | | `--before-layer ` | Where to insert it. | | `--setfilter ` / `--layer-id ` | Filter an existing layer. | @@ -2726,7 +2735,7 @@ both as integers — pass `0 0` rather than `"" ""`. #### Examples ```sh -mapbox static get-image streets-v12 "" 24.94 60.16 12 0 0 600 400 "" .png \ +mapbox static get-image streets-v12 "" 24.94 60.16 12 0 0 600 400 \ --username mapbox > map.png mapbox static get-image streets-v12 "pin-s+555555(24.94,60.16)" \ @@ -2762,9 +2771,10 @@ One raster tile rendered from a style, rather than from a tileset. #### Parameters -Seven positionals: ` `. +Five positionals, then two optional ones: ` +[highRes] [format]`. -`` is `512` (the default) or `256`. 512 px tiles at zoom *z* cover +`` is `512` (the API's default) or `256`, and has to be written. 512 px tiles at zoom *z* cover the same ground as 256 px tiles at *z+1*, so 256 needs four times as many requests for the same area. @@ -2774,7 +2784,7 @@ requests for the same area. #### Examples ```sh -mapbox static get-tile streets-v12 512 2 1 1 "" .png \ +mapbox static get-tile streets-v12 512 2 1 1 \ --username mapbox > tile.png mapbox static get-tile streets-v12 256 12 2048 1361 "@2x" .png \ --username mapbox > tile@2x.png @@ -3301,13 +3311,13 @@ tile. ` ` are the tile coordinates. `` is `png`, `pngraw`, `jpg`, `jpeg` or `webp`. `` is appended to it rather than being a separate URL segment, so `jpg` with `70` -fetches `.jpg70`; pass `""` for the format's default. Both are positional, -in that order. +fetches `.jpg70`; leave it off for the format's default. Both are +positional, in that order. #### Examples ```sh -mapbox tilesets get-tile mapbox.satellite 2 1 1 png "" > tile.png +mapbox tilesets get-tile mapbox.satellite 2 1 1 png > tile.png mapbox tilesets get-tile mapbox.satellite 12 2048 1361 jpg 70 > tile.jpg70 ``` @@ -3331,7 +3341,7 @@ it to a file, e.g. `... > out.jpg`. ``` -$ mapbox tilesets get-tile mapbox.satellite 2 1 1 png "" > tile.png +$ mapbox tilesets get-tile mapbox.satellite 2 1 1 png > tile.png $ file tile.png tile.png: JPEG image data ``` @@ -3410,7 +3420,7 @@ the point. | `--limit ` | How many features. Default 5, max 50. | | `--layers ` | Only these source layers. | | `--geometry ` | Only `point`, `linestring` or `polygon`. | -| `--dedupe` | Drop duplicate features. | +| `--dedupe` | Drop duplicate features. On by default; `--dedupe=false` keeps them. | | `--bands ` | Raster-array bands to sample. | | `--language` / `--worldview` | As in geocoding. | diff --git a/src/account_usage.rs b/src/account_usage.rs index 1566333..23e62e5 100644 --- a/src/account_usage.rs +++ b/src/account_usage.rs @@ -20,7 +20,6 @@ use crate::spec::{parse_spec, Operation}; // `--help` points at the tracker rather than at one issue in it: issue // numbers do not survive a repository move, and a dead number in help text // printed by every shipped binary is worse than no number at all. -use crate::REPO_URL; pub const COMMAND: &str = "usage"; @@ -74,13 +73,12 @@ fn operation() -> &'static Operation { pub fn command() -> Command { Command::new(COMMAND) .about("Show account and token usage by product and day") - .long_about(format!( + .long_about( "Show usage per Mapbox product, by day, for the account or one token.\n\n\ Calls the Statistics API; the token needs the `statistics:read` scope. \ `mapbox auth login` requests it by default; log in again if your stored \ - token predates that.\n\n\ - See {REPO_URL}/issues." - )) + token predates that.", + ) .arg( Arg::new(TOKEN_ID_ARG) .long(TOKEN_ID_ARG) @@ -661,10 +659,9 @@ fn fetch( // out of the request instead. The pinning test below is the actual // guard against that drift. // - // Mirrors `executor::dispatch`'s `is_boolean` branch: today's three - // parameters are all strings, but a future boolean one must not silently - // vanish from the request the way a bare `get_one::` would leave - // it. + // Today's three parameters are all strings, but a future boolean one + // would be a hand-declared switch in `command()`, which a bare + // `get_one::` would silently drop from the request. for param in &operation().query_params { if param.is_boolean { if matches.get_flag(¶m.arg_name) { diff --git a/src/agent_skills.rs b/src/agent_skills.rs index 83d2340..96bcabb 100644 --- a/src/agent_skills.rs +++ b/src/agent_skills.rs @@ -203,7 +203,7 @@ fn ref_arg() -> Arg { .long(REF_ARG) .value_name("REF") .default_value(DEFAULT_REF) - .help("Branch, tag or commit to install from. A commit SHA pins the install") + .help("Branch, tag or commit of the skills repository. A commit SHA pins it") } /// The archive URL for one ref. diff --git a/src/commands.rs b/src/commands.rs index def572a..11cf6c7 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -646,7 +646,7 @@ mod tests { ( "docs.tsv", include_str!("../tests/fixtures/commands_search/docs.tsv"), - 22, + 23, 30, ), ]; diff --git a/src/config.rs b/src/config.rs index dfd9c97..694b2aa 100644 --- a/src/config.rs +++ b/src/config.rs @@ -179,6 +179,12 @@ pub fn command() -> Command { Arg::new("key") .required(true) .value_parser(PossibleValuesParser::new(KEYS)) + // Defaults as `resolve` reads them. + .help( + "update-check: look for a newer version (default on); history: record \ + each run (on); log: keep diagnostic detail for recorded runs, needs \ + history (off); telemetry: send usage events to Mapbox (on)", + ) }; Command::new(COMMAND) diff --git a/src/executor.rs b/src/executor.rs index 38687d0..482d310 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -158,8 +158,15 @@ fn dispatch( } } - for param in &op.path_params { - if let Some(val) = matches.get_one::(¶m.arg_name) { + let omittable = crate::schema::omittable_tail(&op.path_params, &op.path_template); + for (param, omittable) in op.path_params.iter().zip(omittable) { + // An absent one is sent as `""`, which `omittable_tail` vouches for; + // anything else absent falls through to the missing-parameter error. + let val = matches + .get_one::(¶m.arg_name) + .map(String::as_str) + .or(omittable.then_some("")); + if let Some(val) = val { let safe = path_segment_for(&op.service, param, val)?; path = substitute_path_param(&path, ¶m.name, &safe, param.required); } @@ -193,11 +200,7 @@ fn dispatch( } for param in &op.query_params { - if param.is_boolean { - if matches.get_flag(¶m.arg_name) { - query.push((param.name.clone(), "true".to_string())); - } - } else if let Some(val) = matches.get_one::(¶m.arg_name) { + if let Some(val) = matches.get_one::(¶m.arg_name) { query.push((param.name.clone(), val.clone())); } } diff --git a/src/generate_skills.rs b/src/generate_skills.rs index dd659a1..70d9f6e 100644 --- a/src/generate_skills.rs +++ b/src/generate_skills.rs @@ -935,6 +935,9 @@ fn describe(arg: &Argument) -> String { if arg.omit_with_empty { parts.push("Pass an empty string to leave it out of the URL.".to_string()); } + if arg.accepts_false { + parts.push(format!("Write `{}=false` to send false.", spelling(arg))); + } if !arg.conflicts_with.is_empty() { let others: Vec = arg .conflicts_with @@ -1057,6 +1060,7 @@ fn clone_argument(arg: &Argument) -> Argument { required_one_of: arg.required_one_of.clone(), fills: arg.fills.clone(), omit_with_empty: arg.omit_with_empty, + accepts_false: arg.accepts_false, env: arg.env.clone(), default: arg.default.clone(), } diff --git a/src/help_layout.rs b/src/help_layout.rs index f1c7757..020e203 100644 --- a/src/help_layout.rs +++ b/src/help_layout.rs @@ -6,8 +6,9 @@ //! when they serve the same task: //! //! - **Maps and data**: create or fetch map content — styles, sprites, -//! fonts, tiles, static images. -//! - **Search**: find places and addresses, and report problems with them. +//! fonts, tiles, static images — and read or submit reports of wrong map +//! data. +//! - **Search**: find places and addresses. //! - **Navigation**: routes, travel times and reachable areas, and snapping //! GPS traces to roads. //! - **Account**: credentials, tokens and usage — who you are and what you @@ -33,9 +34,11 @@ use clap::{Arg, Command}; const GROUPS: &[(&str, &[&str])] = &[ ( "Maps and data", - &["styles", "sprites", "fonts", "tilesets", "static"], + &[ + "styles", "sprites", "fonts", "tilesets", "static", "feedback", + ], ), - ("Search", &["search", "geocoder", "places", "feedback"]), + ("Search", &["search", "geocoder", "places"]), ( "Navigation", &["directions", "matrix", "isochrone", "map-match"], @@ -84,7 +87,7 @@ const DESCRIPTIONS: &[(&str, &str)] = &[ ), ( "feedback", - "Report wrong or missing map data, and list what was reported", + "Read and submit user reports of wrong or missing map data", ), ("directions", "Route between waypoints"), ( diff --git a/src/main.rs b/src/main.rs index 36c147f..fb0bb13 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,6 +40,7 @@ mod help_layout; mod history; mod http; mod link; +mod markdown; mod mcp; mod output; mod remedy; @@ -61,14 +62,6 @@ use output::{CliError, Mode}; use remedy::Remedy; use spec::ServiceSpec; -/// Where this CLI's source and issue tracker live, written down once so that -/// a repository rename is one edit rather than a grep. -/// -/// The two installers keep a copy of their own each. They are downloaded and -/// run on their own, with nothing of this crate beside them, so this is the -/// single source for the Rust half rather than for all three. -const REPO_URL: &str = repo_url!(); - /// Opens the top-level help when an agent is driving the CLI. Agents explore /// by chaining `--help` calls, one command at a time; `--schema` answers the /// same question for every command in one document. @@ -129,11 +122,8 @@ fn looks_like_a_flag(raw: &str) -> bool { } } -/// A parameter's description, cut to its first sentence for a line of help. -/// -/// The spec text is kept whole by `parse_spec` — `--schema` publishes it and -/// a cut there lands inside identifiers and decimals. Help has a line to -/// work with, so it shortens here, at the only place that wants it. +/// A description cut to its first sentence, for a table cell that has one +/// line to work with. /// /// A sentence ends at a full stop followed by white space, or at the end. /// Cutting at any `.` stopped help inside `{owner}.{tileset}`, `-85.0511` @@ -156,13 +146,66 @@ fn ends_with_abbreviation(text: &str) -> bool { ["e.g.", "i.e."].iter().any(|abbr| text.ends_with(abbr)) } +/// A spec description followed by the page that has the rest of it: help +/// keeps only the opening prose (see [`markdown::intro`]). +fn with_docs_link(text: String, service: &str) -> String { + match remedy::docs_for_service(service) { + Some(url) => format!("{text}\n\nDocs: {url}"), + None => text, + } +} + +/// A parameter's whole description, as one line of help. +/// +/// Whole rather than its first sentence: the second is where a spec says +/// which values are allowed and what else the parameter needs ("Requires +/// `overview=full`", "Required, here or in `--data`"), and help cut there +/// left a caller to find that out from the API's error. The spec's own line +/// breaks are folded, since clap wraps the text itself. fn help_text(param: &spec::Parameter) -> String { param .description .as_deref() - .map(first_sentence) + .map(help_line) .unwrap_or_default() - .to_string() +} + +/// Rendered, and without the closing full stop that clap's own help lines +/// leave off. +fn help_line(text: &str) -> String { + let text = markdown::render(text); + match text.strip_suffix('.') { + Some(stripped) => stripped.to_string(), + None => text, + } +} + +/// The API's value for a spec-optional path parameter that clap cannot take +/// `""` for, so a caller who wants the default knows what to write. The +/// specs say these in prose (`static get-image`'s description, `tilesize`'s +/// `default: 512`), which the parser does not read for a path parameter. +const PATH_PARAM_DEFAULTS: &[(&str, &str)] = + &[("bearing", "0"), ("pitch", "0"), ("tilesize", "512")]; + +/// A positional's help, plus how to skip it when the spec calls it optional +/// but clap still needs something written in its place. +fn path_help_text(param: &spec::Parameter, path_template: &str, omittable: bool) -> String { + let help = help_text(param); + let how_to_skip = if param.required || omittable { + None + } else if schema::omits_with_empty(param, path_template) { + Some("Pass \"\" to leave it out".to_string()) + } else { + PATH_PARAM_DEFAULTS + .iter() + .find(|(name, _)| *name == param.name) + .map(|(_, default)| format!("Pass {default} for the API's default")) + }; + match how_to_skip { + Some(note) if help.is_empty() => note, + Some(note) => format!("{help}. {note}"), + None => help, + } } /// The coordinate-shaped parameters whose value legitimately starts with `-` @@ -223,6 +266,7 @@ fn build_operation_command(op: &spec::Operation) -> Command { } if let Some(desc) = &op.description { + let desc = with_docs_link(markdown::render(markdown::intro(desc)), &op.service); // Marked here too, not only in `about`. Clap renders `long_about` // for `--help` and `about` for `-h`, so a command with a spec // description — all 54 of them have one — showed the marker in its @@ -231,15 +275,16 @@ fn build_operation_command(op: &spec::Operation) -> Command { cmd = cmd.long_about(if op.deprecated { format!("[deprecated] {desc}") } else { - desc.clone() + desc }); } - for param in &op.path_params { + let omittable = schema::omittable_tail(&op.path_params, &op.path_template); + for (param, omittable) in op.path_params.iter().zip(omittable) { let mut arg = Arg::new(param.arg_name.clone()) - .required(true) + .required(!omittable) .allow_hyphen_values(takes_hyphen_leading_value(param)) - .help(help_text(param)); + .help(path_help_text(param, &op.path_template, omittable)); // Positionals get the same treatment as flags: clap renders the // possible values into the help itself, so the hand-rolled suffix @@ -257,9 +302,19 @@ fn build_operation_command(op: &spec::Operation) -> Command { for param in &op.query_params { let arg = if param.is_boolean { + // Present means `true`, as before, and `--flag=false` sends + // `false`: several of these default to true at the API + // (`attribution`, `autocomplete`, `dedupe`), so a bare switch + // could only ever send the default. The `=` is required so the + // optional value cannot take the next positional for itself. Arg::new(param.arg_name.clone()) .long(param.arg_name.clone()) - .action(ArgAction::SetTrue) + .num_args(0..=1) + .require_equals(true) + .default_missing_value("true") + .value_name("true|false") + .value_parser(PossibleValuesParser::new(["true", "false"])) + .hide_possible_values(true) .help(help_text(param)) } else { let mut arg = Arg::new(param.arg_name.clone()) @@ -378,9 +433,8 @@ fn build_operation_command(op: &spec::Operation) -> Command { let help = variadic .description .as_deref() - .map(first_sentence) - .unwrap_or_default() - .to_string(); + .map(help_line) + .unwrap_or_default(); cmd = cmd.arg( Arg::new(variadic.arg_name.clone()) .required(true) @@ -493,7 +547,7 @@ fn build_service_command(svc: &ServiceSpec) -> Command { .subcommand_required(true); if let Some(desc) = &svc.description { - cmd = cmd.long_about(desc.clone()); + cmd = cmd.long_about(with_docs_link(markdown::render(desc), &svc.name)); } attach_operations(cmd, &svc.name, &exposed, 0) @@ -543,7 +597,7 @@ fn build_app(specs: &[ServiceSpec]) -> Command { .value_name("PROFILE") .help_heading(AUTH_OPTIONS) .global(true) - .help("Credential profile to use (default: \"default\")") + .help("Credential profile to use [default: default]") .required(false), ) .arg( @@ -633,7 +687,7 @@ fn build_app(specs: &[ServiceSpec]) -> Command { .value_name("ID") .help_heading(OUTPUT_OPTIONS) .global(true) - .help("Show only the row with this id from a list"), + .help("Show only the row with this id from an API listing"), ) .arg( Arg::new(output::ARG) @@ -663,7 +717,7 @@ fn build_app(specs: &[ServiceSpec]) -> Command { .action(ArgAction::SetTrue) .help_heading(OUTPUT_OPTIONS) .global(true) - .help("Turn off color, as NO_COLOR does"), + .help("Turn off color [env: NO_COLOR]"), ) .arg( Arg::new(schema::ARG) @@ -1335,6 +1389,21 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - deprecation::warn_command(&path); } + // `--id` is global so that every API command takes it, but only the + // executor reads it. Anywhere else it was ignored, and `history show + // --id x` printed the newest run as if it were `x`. + if matches.get_one::(output::FILTER_ARG).is_some() + && !matches + .subcommand_name() + .is_some_and(|name| specs.iter().any(|svc| svc.name == name)) + { + return Err(CliError::new( + "usage", + "`--id` only applies to an API command that returns a list.", + ) + .into()); + } + match matches.subcommand() { // Ahead of the generic service arm, which would otherwise catch this // as an unknown service. `tilesets` makes its own HTTP calls, so the @@ -2319,7 +2388,17 @@ mod tests { /// keeps the paragraph, and this is where it gets cut back — and the /// sentence ends at a full stop, not at a dot inside an id or a number. #[test] - fn help_shortens_a_description_to_its_first_sentence() { + fn help_is_the_whole_description_on_one_line() { + assert_eq!( + help_line("A layer to add to the map\nbefore rendering. Maximum one per request."), + "A layer to add to the map before rendering. Maximum one per request" + ); + assert_eq!(help_line(" no full stop here "), "no full stop here"); + assert_eq!(help_line(""), ""); + } + + #[test] + fn first_sentence_stops_at_a_real_full_stop() { assert_eq!( first_sentence("Tileset ID in the format `username.id`. Order matters."), "Tileset ID in the format `username.id`" @@ -2522,6 +2601,61 @@ mod tests { assert_eq!(seen, expected, "auth lost a subcommand"); } + /// The specs' descriptions are Markdown written for docs.mapbox.com, and + /// none of that markup may reach `--help`: no heading, table, fence, + /// ``, bold marker or bare link syntax, on any command or service. + #[test] + fn no_markdown_reaches_help() { + let specs = bundled_specs(); + let parsing = help_layout::for_parsing(help_layout::apply(build_app(&specs))); + let mut paths: Vec> = leaf_commands(&parsing, &[]) + .into_iter() + .map(|(path, _)| path) + .collect(); + paths.extend(specs.iter().map(|svc| vec![svc.name.clone()])); + + let mut problems = vec![]; + for path in paths { + let argv: Vec<&str> = std::iter::once("mapbox") + .chain(path.iter().map(String::as_str)) + .chain(["--help"]) + .collect(); + let Err(help) = parsing.clone().try_get_matches_from(&argv) else { + continue; + }; + let help = help.render().to_string(); + for marker in ["## ", "**", "```", "` blocks around the prose. Printed as they are, that +//! made `geocoder batch --help` 113 lines of markup, most of it about calling +//! the API by hand. [`intro`] keeps the opening prose, and [`render`] turns +//! its markup into plain text. +//! +//! Only spec text comes through here. Hand-written help is plain text with +//! its own line breaks, which Markdown would fold into one paragraph. +//! `--schema` keeps the source: an agent reads Markdown well and wants it +//! whole. + +use pulldown_cmark::{Event, Options, Parser, Tag, TagEnd}; + +/// The opening prose of `text`, as Markdown: paragraphs and lists up to the +/// first heading, code block, table, quote, rule or HTML block. What follows +/// one of those is reference material — option tables, worked examples, +/// billing notes — that the page linked from help covers in full. +pub fn intro(text: &str) -> &str { + let mut depth = 0usize; + let mut end = 0; + for (event, range) in Parser::new_ext(text, Options::ENABLE_TABLES).into_offset_iter() { + match event { + Event::Start(tag) => { + if depth == 0 && !matches!(tag, Tag::Paragraph | Tag::List(_)) { + break; + } + depth += 1; + } + Event::End(_) => { + depth -= 1; + if depth == 0 { + end = range.end; + } + } + Event::Html(_) | Event::Rule if depth == 0 => break, + _ => {} + } + } + text[..end].trim_end() +} + +/// `text` with its Markdown rendered as plain text. +/// +/// Plain rather than styled: clap's wrapping doubles every line break in a +/// paragraph that opens with a styled run right after a blank line (clap +/// 4.6), which is how `tilesets get-mvt`'s "**Style-optimized tiles**:" +/// paragraph came out. Markers are dropped, a link keeps its address in +/// parentheses since a terminal may not make it clickable, paragraphs are +/// separated by a blank line and list items start their own line. +pub fn render(text: &str) -> String { + let mut out = String::new(); + let mut lists: Vec> = vec![]; + let mut links: Vec = vec![]; + let mut link_text_start = 0; + + for event in Parser::new_ext(text, Options::ENABLE_TABLES) { + match event { + Event::Start(Tag::Paragraph) | Event::Start(Tag::Heading { .. }) + if lists.is_empty() => + { + separate(&mut out, "\n\n"); + } + Event::End(TagEnd::Heading(_)) => out.push('\n'), + Event::Start(Tag::List(first)) => { + if lists.is_empty() { + separate(&mut out, "\n"); + } + lists.push(first); + } + Event::End(TagEnd::List(_)) => { + lists.pop(); + } + Event::Start(Tag::Item) => { + separate(&mut out, "\n"); + out.push_str(&" ".repeat(lists.len().saturating_sub(1))); + match lists.last_mut() { + Some(Some(n)) => { + out.push_str(&format!("{n}. ")); + *n += 1; + } + _ => out.push_str("- "), + } + } + Event::Code(code) => out.push_str(&code), + Event::Start(Tag::Link { dest_url, .. }) => { + links.push(dest_url.to_string()); + link_text_start = out.len(); + } + Event::End(TagEnd::Link) => { + if let Some(url) = links.pop() { + if out[link_text_start..] != url { + out.push_str(&format!(" ({url})")); + } + } + } + Event::Start(Tag::CodeBlock(_)) => separate(&mut out, "\n\n"), + Event::Text(text) => out.push_str(&text), + Event::SoftBreak => out.push(' '), + Event::HardBreak => out.push('\n'), + _ => {} + } + } + out.trim().to_string() +} + +/// Ends what came before with `gap` — unless nothing did, or it already ends +/// that way. +fn separate(out: &mut String, gap: &str) { + let trimmed = out.trim_end_matches(' ').len(); + out.truncate(trimmed); + if out.is_empty() || out.ends_with(gap) { + return; + } + while out.ends_with('\n') { + out.pop(); + } + out.push_str(gap); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn plain(text: &str) -> String { + render(text) + } + + #[test] + fn the_intro_stops_at_the_first_reference_block() { + let text = "Looks up a place.\n\n## Structured input\n\nMore."; + assert_eq!(intro(text), "Looks up a place."); + + let text = "Up to 50 queries.\n\n\n Each counts.\n\n"; + assert_eq!(intro(text), "Up to 50 queries."); + + let text = "Run it:\n\n```bash\ncurl …\n```\n"; + assert_eq!(intro(text), "Run it:"); + } + + /// A paragraph that ends in a colon keeps the list it introduces. + #[test] + fn the_intro_keeps_a_list() { + let text = "Three kinds:\n\n - **Public** (`pk`)\n - **Secret** (`sk`)\n\n| a | b |\n|---|---|\n| 1 | 2 |\n"; + assert_eq!( + intro(text), + "Three kinds:\n\n - **Public** (`pk`)\n - **Secret** (`sk`)" + ); + } + + #[test] + fn inline_markup_is_rendered_rather_than_printed() { + assert_eq!( + plain("The **Geocoding API** does _two_ things\nwith `q`."), + "The Geocoding API does two things with q." + ); + assert_eq!( + plain("See [Slippy Map Tilenames](https://wiki.example/x)."), + "See Slippy Map Tilenames (https://wiki.example/x)." + ); + // The code spans a regex stripped into "and or x". + assert_eq!(plain("`and` or `x`"), "and or x"); + } + + #[test] + fn paragraphs_and_lists_keep_their_shape() { + assert_eq!( + plain("Two things:\n\n- Forward\n- Reverse\n\nAfter."), + "Two things:\n- Forward\n- Reverse\n\nAfter." + ); + assert_eq!(plain("Steps:\n\n1. One\n2. Two"), "Steps:\n1. One\n2. Two"); + } +} diff --git a/src/mcp.rs b/src/mcp.rs index 5b22d92..c49638c 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -352,17 +352,18 @@ pub fn command() -> ClapCommand { belongs to the agent and may already list other servers, so this shells out to \ the agent's own tooling (`claude mcp add`, `codex mcp add`, `code --add-mcp`) \ rather than editing that store directly.\n\n\ - Against the hosted Mapbox MCP endpoints only — no token, no npm package, no Node \ - version to manage. `mapbox mcp list` names every server and client this command \ + Uses the hosted Mapbox MCP endpoints: no Mapbox token to paste, no npm package, \ + no Node version to manage. The server asks the agent to sign in on first use. \ + `mapbox mcp list` names every server and client this command \ knows about.", ) .subcommand_required(true) .subcommand( - ClapCommand::new(LIST).about("List known MCP servers and clients, and what's already installed"), + ClapCommand::new(LIST).about("List known MCP servers and clients, and which are already registered"), ) .subcommand( ClapCommand::new(INSTALL) - .about("Register a server with a client. With neither flag, every known server for every detected client") + .about("Register MCP servers with coding agents; with no flags, every known server with every detected agent") .long_about(format!( "Register the {} servers with a coding agent: {}. With neither flag, registers \ every known server with every detected client.", diff --git a/src/schema.rs b/src/schema.rs index b46f2ad..9b8d68a 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -60,9 +60,22 @@ pub(crate) struct Schema { target: String, pub(crate) authentication: &'static str, pub(crate) global_options: Vec, + /// The commands that hold others — `mapbox history`, `mapbox geocoder` — + /// with what their own help says. Only leaves are in `commands`, and the + /// group's text is often the part that applies to all of them: how long + /// history is kept, that `config` writes a file. + pub(crate) groups: Vec, pub(crate) commands: Vec, } +#[derive(serde::Serialize)] +pub(crate) struct Group { + pub(crate) command: String, + pub(crate) summary: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) description: Option, +} + #[derive(serde::Serialize)] pub(crate) struct Cli { pub(crate) name: &'static str, @@ -186,11 +199,16 @@ pub(crate) struct Argument { pub(crate) fills: Option, /// Whether passing an empty string leaves this positional out of the /// URL — usually a suffix (`.png` off the end of a tile name), sometimes - /// a whole segment. The argument must still be written: clap requires - /// every path positional, and the spec's optional ones are omitted by - /// value, not by absence. + /// a whole segment. Only the run of these at the end of the line may be + /// left off altogether (`required: false`); one before a required + /// positional still has to be written, as `""`. #[serde(skip_serializing_if = "is_false")] pub(crate) omit_with_empty: bool, + /// Whether this flag also takes `--name=false`. Present alone it sends + /// `true`; the spelling with `=false` is the only way to send `false`, + /// which matters where the API defaults to `true`. + #[serde(skip_serializing_if = "is_false")] + pub(crate) accepts_false: bool, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) env: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -223,6 +241,7 @@ impl Argument { required_one_of: vec![], fills: None, omit_with_empty: false, + accepts_false: false, env: None, default: None, } @@ -311,10 +330,68 @@ pub(crate) fn build(app: &Command, specs: &[ServiceSpec], path: &[String]) -> Sc // Whole, at every level: a caller that asked about one command still // needs to know a token goes on the front of it. global_options: global_options(app), + groups: groups(app, specs, path), commands: commands(app, specs, path), } } +/// Every group on the way to the target or under it: `mapbox history list +/// --schema` still gets `mapbox history`, whose text covers it. +fn groups(app: &Command, specs: &[ServiceSpec], path: &[String]) -> Vec { + let mut out = vec![]; + collect_groups(app, specs, &mut vec![], path, &mut out); + out +} + +fn collect_groups( + cmd: &Command, + specs: &[ServiceSpec], + at: &mut Vec, + target: &[String], + out: &mut Vec, +) { + let children = || { + cmd.get_subcommands() + .filter(|sub| sub.get_name() != "help" && !sub.is_hide_set()) + }; + if !at.is_empty() && children().next().is_some() { + let related = at.starts_with(target) || target.starts_with(at); + if related { + out.push(group(cmd, specs, at)); + } + } + for sub in children() { + at.push(sub.get_name().to_string()); + collect_groups(sub, specs, at, target, out); + at.pop(); + } +} + +fn group(cmd: &Command, specs: &[ServiceSpec], at: &[String]) -> Group { + let service = (at.len() == 1) + .then(|| specs.iter().find(|svc| svc.name == at[0])) + .flatten(); + Group { + command: std::iter::once("mapbox") + .chain(at.iter().map(String::as_str)) + .collect::>() + .join(" "), + // An API service's `about` is its spec's title ("Mapbox Tokens + // API"); the help listing's line says what the group is for. + summary: crate::help_layout::description(&at[0]) + .filter(|_| at.len() == 1) + .map(String::from) + .or_else(|| cmd.get_about().map(ToString::to_string)) + .unwrap_or_default(), + // A service's spec Markdown rather than the help rendered from it, + // the same as an API command's `description`. + description: match service { + Some(svc) => svc.description.clone(), + None => long_about(cmd), + }, + } +} + fn global_options(app: &Command) -> Vec { app.get_arguments() .filter(|arg| arg.is_global_set()) @@ -448,17 +525,17 @@ fn declared_command<'a>(app: &'a Command, service: &str, path: &[String]) -> Opt fn api_command(app: &Command, svc: &ServiceSpec, op: &Operation) -> CommandEntry { let declared = declared_command(app, &svc.name, &op.command_path); + let omittable = omittable_tail(&op.path_params, &op.path_template); let mut arguments: Vec = op .path_params .iter() - .map(|param| Argument { - // Every path positional is required, whatever the spec says, - // because `build_operation_command` makes it so: the value has to - // be written even where the spec calls the segment optional. What - // the spec's `required: false` buys is the right to write nothing - // *as* that value, which `omit_with_empty` reports rather than - // contradicting clap here. - required: true, + .enumerate() + .map(|(index, param)| Argument { + // Clap's answer, not the spec's: a spec-optional positional can + // only be left off at the end of the line, so one earlier on is + // still required here and `omit_with_empty` reports whether `""` + // stands in for it. + required: !omittable[index], location: Some("path"), omit_with_empty: omits_with_empty(param, &op.path_template), // Named as well as filled, the same reason `username_argument` @@ -483,6 +560,7 @@ fn api_command(app: &Command, svc: &ServiceSpec, op: &Operation) -> CommandEntry flag: Some(format!("--{}", param.arg_name)), required: param.required, location: Some("query"), + accepts_false: param.is_boolean, ..parameter(param, kind) } })); @@ -582,7 +660,7 @@ fn parameter(param: &Parameter, kind: ArgKind) -> Argument { /// (`bearing`, `pitch`) or for an enum that does not list `""` among its /// values (`tilesize`) — those are spec-optional and, through this CLI, /// impossible to omit. And the URL that results has to be a URL. -fn omits_with_empty(param: &Parameter, path_template: &str) -> bool { +pub(crate) fn omits_with_empty(param: &Parameter, path_template: &str) -> bool { if param.required { return false; } @@ -596,6 +674,22 @@ fn omits_with_empty(param: &Parameter, path_template: &str) -> bool { clap_accepts_empty && leaves_a_usable_url(path_template, ¶m.name) } +/// Which path positionals may be left off the command line altogether. +/// +/// Clap can only drop a positional when every one after it can go too, so +/// this is the run at the end that [`omits_with_empty`] allows: the executor +/// sends an absent one exactly as it sends `""`. +pub(crate) fn omittable_tail(path_params: &[Parameter], path_template: &str) -> Vec { + let mut omittable = vec![false; path_params.len()]; + for (index, param) in path_params.iter().enumerate().rev() { + if !omits_with_empty(param, path_template) { + break; + } + omittable[index] = true; + } + omittable +} + /// Whether taking this placeholder out of the template leaves a path the API /// will read the way the caller meant. /// @@ -864,6 +958,7 @@ fn long_about(cmd: &Command) -> Option { /// command declares. fn clap_argument(arg: &Arg) -> Argument { let long = arg.get_long(); + let help = arg.get_help().map(ToString::to_string); let is_flag = matches!(arg.get_action(), ArgAction::SetTrue | ArgAction::SetFalse); let kind = match (long.is_some(), is_flag) { @@ -896,9 +991,9 @@ fn clap_argument(arg: &Arg) -> Argument { .map(|value| value.get_name().to_string()) .collect() }, - description: arg - .get_help() - .map(ToString::to_string) + description: help + .as_deref() + .map(without_hand_notes) .filter(|help| !help.is_empty()), // `--file` repeats by being written again; `tilesets-cli`'s // passthrough takes many values at once. Both mean "more than one". @@ -906,12 +1001,17 @@ fn clap_argument(arg: &Arg) -> Argument { || arg .get_num_args() .is_some_and(|range| range.max_values() > 1), - env: arg.get_env().and_then(|env| env.to_str()).map(String::from), + env: arg + .get_env() + .and_then(|env| env.to_str()) + .map(String::from) + .or_else(|| hand_note(help.as_deref(), "env")), default: arg .get_default_values() .first() .and_then(|value| value.to_str()) - .map(String::from), + .map(String::from) + .or_else(|| hand_note(help.as_deref(), "default")), ..Argument::new( long.unwrap_or_else(|| arg.get_id().as_str()).to_string(), kind, @@ -919,6 +1019,39 @@ fn clap_argument(arg: &Arg) -> Argument { } } +/// A `[env: X]` or `[default: X]` written into an argument's help by hand. +/// +/// Some arguments read their variable or apply their default in code rather +/// than through clap, so that an unusable `MAPBOX_OUTPUT=` or +/// `MAPBOX_TIMEOUT=30s` warns instead of failing every command. Their help +/// spells the note the way clap would, and this reads it back, so the +/// schema's `env` and `default` hold for every argument rather than only +/// those clap knows about. +fn hand_note(help: Option<&str>, key: &str) -> Option { + let open = format!("[{key}: "); + let start = help?.find(&open)? + open.len(); + let rest = &help?[start..]; + Some(rest[..rest.find(']')?].to_string()) +} + +/// `help` without the notes [`hand_note`] reads, which the schema reports +/// as fields instead. +fn without_hand_notes(help: &str) -> String { + let mut text = help.to_string(); + for key in ["env", "default"] { + let open = format!("[{key}: "); + if let Some(start) = text.find(&open) { + if let Some(len) = text[start..].find(']') { + text.replace_range(start..=start + len, ""); + } + } + } + text.split_whitespace() + .collect::>() + .join(" ") + .replace(" .", ".") +} + #[cfg(test)] mod tests { use super::*; @@ -1348,6 +1481,47 @@ mod tests { )); } + /// Only the run at the end can be left off: clap cannot skip a + /// positional with a required one after it. + #[test] + fn only_the_trailing_run_of_omittable_positionals_is_optional() { + let required = |name| Parameter { + required: true, + ..optional(name) + }; + let params = [ + optional("overlay"), + required("zoom"), + required("w"), + required("h"), + optional("highRes"), + optional("format"), + ]; + assert_eq!( + omittable_tail(¶ms, "/static/{overlay}/{zoom}/{w}x{h}{highRes}{format}"), + [false, false, false, false, true, true] + ); + } + + #[test] + fn a_hand_written_note_becomes_a_field() { + let help = "GitHub repository to install from [default: mapbox/mapbox-cli]. Reads \ + GH_TOKEN for a private one"; + assert_eq!( + hand_note(Some(help), "default").as_deref(), + Some("mapbox/mapbox-cli") + ); + assert_eq!(hand_note(Some(help), "env"), None); + assert_eq!( + without_hand_notes(help), + "GitHub repository to install from. Reads GH_TOKEN for a private one" + ); + assert_eq!( + without_hand_notes("Seconds per request [env: MAPBOX_TIMEOUT]"), + "Seconds per request" + ); + } + /// A schema is JSON even when the caller asked for text, and indented /// only when someone is looking at it. #[test] diff --git a/src/skill_dest.rs b/src/skill_dest.rs index 4da2f4b..5ceba86 100644 --- a/src/skill_dest.rs +++ b/src/skill_dest.rs @@ -537,19 +537,19 @@ pub fn args() -> Vec { .value_parser(PossibleValuesParser::new(flags)) .conflicts_with(DIR_ARG) .help( - "Agent to write skills for, repeatable. Defaults to whichever \ - agents are installed", + "Agent whose skills to use, repeatable. Defaults to every \ + agent whose home directory is present", ), Arg::new(GLOBAL_ARG) .long(GLOBAL_ARG) .action(ArgAction::SetTrue) .conflicts_with(DIR_ARG) - .help("Write to the agent's home directory, for every project, rather than this one"), + .help("Use the agent's home directory, for every project, rather than this one"), Arg::new(DIR_ARG) .long(DIR_ARG) .value_name("DIR") .value_parser(clap::value_parser!(PathBuf)) - .help("Write to this directory instead, and nowhere else"), + .help("Use this directory instead, and nowhere else"), ] } diff --git a/src/spec.rs b/src/spec.rs index ec9e189..57836c7 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -1086,12 +1086,11 @@ pub fn parse_spec(service_name: &str, yaml: &str) -> Result { .unwrap_or(service_name) .to_string(); - // Flattened to one line (unlike a parameter's description): this - // renders as a service's `long_about`, where the spec's own line - // wrapping doesn't help. + // The opening prose, as Markdown: a first paragraph alone stopped + // `accounts` at "There are three kinds of tokens:", before the list. let description = doc["info"]["description"] .as_str() - .map(|text| first_paragraph(text).replace('\n', " ")); + .map(|text| crate::markdown::intro(text).to_string()); let base_url = doc["servers"] .as_sequence() @@ -1669,8 +1668,6 @@ fn parse_parameter(val: &Value) -> Option { // extreme: `geocoder --types` alone is 4.6 KB of feature-type // reference, and the schema gets read on every call. A paragraph // avoids both problems, and leaves the appendices out. - // `crate::first_sentence` shortens it further for `--help`, which - // still renders exactly as it always has. let description = val["description"].as_str().map(first_paragraph); let schema = &val["schema"]; diff --git a/src/uninstall.rs b/src/uninstall.rs index be90736..bfa495d 100644 --- a/src/uninstall.rs +++ b/src/uninstall.rs @@ -34,9 +34,10 @@ pub fn command() -> Command { .about("Uninstall this CLI by removing the installed `mapbox` binary") .long_about( "Remove the `mapbox` binary this process is running from.\n\n\ - Deletes only the executable. Stored credentials, `auth logout`'s file, \ - and the separately-installed `tilesets` binary are left alone — run \ - `mapbox auth logout` first if you also want the credential store gone.", + Deletes only the executable. The stored credentials, settings, run history, \ + logs and installed workflows in `~/.mapbox` stay, as does the \ + separately-installed `tilesets` binary — run `mapbox auth logout` first to \ + remove the stored credentials.", ) .arg(executor::dry_run_arg(DRY_RUN_HELP)) } diff --git a/src/workflow/runner.rs b/src/workflow/runner.rs index 2115995..437d644 100644 --- a/src/workflow/runner.rs +++ b/src/workflow/runner.rs @@ -678,13 +678,21 @@ fn find_arg<'a>( }) .ok_or_else(|| format!("`{key}` is not an argument of this command"))?; let repeatable = matches!(arg.get_action(), ArgAction::Append); - let shape = if matches!(arg.get_action(), ArgAction::SetTrue | ArgAction::SetFalse) { - ArgShape::Flag - } else if arg.is_positional() { - ArgShape::Positional { repeatable } - } else { - ArgShape::Option { repeatable } - }; + // A generated boolean flag also takes `=false`, but a step's `false` + // still leaves it out, so existing workflows send the same requests. + // Sending `x=false` instead would add a parameter to every request that + // wrote `false`, and nothing has checked each API accepts that. The cost + // is that a step cannot turn off a flag that defaults to true; the + // README says so. + let takes_false = arg.is_require_equals_set() && arg.get_num_args() == Some((0..=1).into()); + let shape = + if takes_false || matches!(arg.get_action(), ArgAction::SetTrue | ArgAction::SetFalse) { + ArgShape::Flag + } else if arg.is_positional() { + ArgShape::Positional { repeatable } + } else { + ArgShape::Option { repeatable } + }; Ok((arg, shape)) } diff --git a/tests/output_contract.rs b/tests/output_contract.rs index 8f25d72..fb14697 100644 --- a/tests/output_contract.rs +++ b/tests/output_contract.rs @@ -1047,7 +1047,147 @@ fn a_missing_argument_error_names_the_arguments() { .to_string(); assert!(message.contains(""), "{message}"); - assert!(message.contains(""), "{message}"); + assert!(message.contains(""), "{message}"); +} + +/// A parameter's help is its whole description: the sentences after the +/// first are where a spec lists the values and what else the parameter +/// needs. Whitespace is folded, since clap wraps the text at its own width. +#[test] +fn help_shows_a_parameter_s_whole_description() { + let help = |args: &[&str]| { + let out = run(args); + assert!(out.status.success(), "{args:?}: {}", stderr(&out)); + stdout(&out) + .split_whitespace() + .collect::>() + .join(" ") + }; + + let matrix = help(&["matrix", "--help"]); + assert!( + matrix.contains("Options are duration (the default) and distance"), + "{matrix}" + ); + let directions = help(&["directions", "--help"]); + assert!( + directions.contains("Requires overview=full"), + "{directions}" + ); + let feedback = help(&["feedback", "create", "--help"]); + assert!( + feedback.contains("Required, here or in --data"), + "{feedback}" + ); +} + +/// A spec's description stops at its reference material, keeps a list its +/// opening introduces, and ends with the page that has the rest. +#[test] +fn help_keeps_a_spec_description_s_opening_prose() { + let forward = stdout(&run(&["geocoder", "forward", "--help"])); + assert!(!forward.contains("Structured Input"), "{forward}"); + assert!( + forward.contains("Docs: https://docs.mapbox.com/api/search/geocoding-v6/"), + "{forward}" + ); + + let accounts = stdout(&run(&["accounts", "--help"])); + assert!(accounts.contains("- Temporary tokens (tk)"), "{accounts}"); +} + +/// Only the executor reads `--id`, so any other command refuses it rather +/// than printing everything as if it had matched. +#[test] +fn id_is_refused_where_nothing_reads_it() { + for args in [ + &["-o", "json", "history", "show", "--id", "nope"][..], + &["-o", "json", "config", "list", "--id", "nope"], + ] { + let out = run(args); + assert_eq!(out.status.code(), Some(2), "{args:?}: {}", stderr(&out)); + assert!(stdout(&out).is_empty(), "{args:?}: {}", stdout(&out)); + assert_eq!(json(&stderr(&out))["code"], "usage", "{args:?}"); + } +} + +/// The URL a command would request, read off `--debug`. The proxy is a dead +/// port, so nothing reaches Mapbox. +fn debug_url(args: &[&str]) -> String { + let out = command() + .env("HTTPS_PROXY", "http://127.0.0.1:9") + .env_remove("https_proxy") + .env_remove("ALL_PROXY") + .env_remove("all_proxy") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .args(["--debug", "--token", "pk.test", "-u", "mapbox"]) + .args(args) + .output() + .expect("run mapbox"); + let err = stderr(&out); + err.lines() + .find_map(|line| line.strip_prefix("[debug] GET ")) + .unwrap_or_else(|| panic!("no request in {err}")) + .to_string() +} + +const STATIC_IMAGE: &[&str] = &[ + "static", + "get-image", + "streets-v12", + "", + "24.94", + "60.17", + "12", + "0", + "0", + "600", + "400", +]; + +/// A spec-optional positional at the end of the line can be left off, and +/// goes out the way `""` would: no placeholder and no stray separator. +#[test] +fn a_trailing_optional_positional_can_be_left_off() { + let url = debug_url(STATIC_IMAGE); + assert!(url.contains("/static/24.94,60.17,12,0,0/600x400?"), "{url}"); +} + +/// `attribution` and `logo` default to true at the API, so a flag that could +/// only send `true` could never take them off the image. +#[test] +fn a_boolean_flag_sends_false_when_told_to() { + let url = debug_url(&[STATIC_IMAGE, &["--attribution=false", "--logo"]].concat()); + assert!(url.contains("attribution=false"), "{url}"); + assert!(url.contains("logo=true"), "{url}"); +} + +/// With the `=` required, `false` after a space is not the flag's value, so +/// the optional value cannot swallow the next positional either. +#[test] +fn a_boolean_flag_value_needs_the_equals_sign() { + let out = command() + .args(["geocoder", "forward", "--q", "x", "--autocomplete", "false"]) + .output() + .expect("run mapbox"); + assert_eq!(out.status.code(), Some(2), "{}", stderr(&out)); +} + +/// One value per coordinate: a single switch is a 422 at the API. +#[test] +fn directions_snapping_takes_a_list_per_coordinate() { + let url = debug_url(&[ + "directions", + "mapbox/driving-traffic", + "24.94,60.17;24.95,60.18", + "--snapping-include-closures", + "true;true", + ]); + assert!( + url.contains("snapping_include_closures=true%3Btrue"), + "{url}" + ); } /// A live 401, carrying the advice a caller acts on: what to do, a command diff --git a/tests/schema_contract.rs b/tests/schema_contract.rs index 4be552c..9b506d7 100644 --- a/tests/schema_contract.rs +++ b/tests/schema_contract.rs @@ -120,6 +120,88 @@ fn a_command_describes_itself_without_being_able_to_run() { assert_eq!(style_id["location"], "path"); } +/// An argument that reads its variable or its default by hand still +/// reports it in `env` and `default`, and the note its help spells it with +/// is not left in the description. +#[test] +fn every_env_and_default_is_a_field() { + let value = schema(&["--schema"]); + let global = |name: &str| { + value["global_options"] + .as_array() + .expect("global options") + .iter() + .find(|arg| arg["name"] == name) + .unwrap_or_else(|| panic!("`{name}` is described")) + .clone() + }; + assert_eq!(global("timeout")["env"], "MAPBOX_TIMEOUT"); + assert_eq!(global("output")["env"], "MAPBOX_OUTPUT"); + assert_eq!(global("no-color")["env"], "NO_COLOR"); + assert_eq!(global("profile")["default"], "default"); + + let arguments = value["global_options"] + .as_array() + .into_iter() + .flatten() + .chain( + commands(&value) + .iter() + .flat_map(|command| command["arguments"].as_array().into_iter().flatten()), + ); + for arg in arguments { + let description = arg["description"].as_str().unwrap_or_default(); + assert!( + !description.contains("[env: ") && !description.contains("[default: "), + "`{}` keeps a note the schema should report as a field: {description}", + arg["name"] + ); + } +} + +/// Only leaves are commands, so what a group's own help says lives in +/// `groups` — including for a target below it. +#[test] +fn a_group_s_own_description_is_described() { + let value = schema(&["history", "list", "--schema"]); + let groups = value["groups"].as_array().expect("groups"); + assert_eq!(groups.len(), 1, "{groups:?}"); + assert_eq!(groups[0]["command"], "mapbox history"); + assert!( + groups[0]["description"] + .as_str() + .unwrap_or_default() + .contains("kept for 30 days"), + "{groups:?}" + ); +} + +/// What an agent needs to write `static get-image` without guessing: which +/// positionals it may leave off, and that a boolean flag also takes `=false`. +#[test] +fn optional_positionals_and_false_flags_are_described() { + let value = schema(&["static", "get-image", "--schema"]); + let arguments = commands(&value)[0]["arguments"] + .as_array() + .expect("arguments"); + let arg = |name: &str| { + arguments + .iter() + .find(|arg| arg["name"] == name) + .unwrap_or_else(|| panic!("`{name}` is described")) + }; + + // Last on the line, so it can go. + assert_eq!(arg("format")["required"], false); + assert_eq!(arg("format")["omit_with_empty"], true); + // Before a required positional, so `""` has to be written. + assert_eq!(arg("overlay")["required"], true); + assert_eq!(arg("overlay")["omit_with_empty"], true); + + assert_eq!(arg("attribution")["kind"], "flag"); + assert_eq!(arg("attribution")["accepts_false"], true); +} + #[test] fn the_account_the_url_needs_is_listed_as_an_argument() { // `{username}` has no parameter of its own — the executor fills it from diff --git a/workflow/README.md b/workflow/README.md index f5a07e4..f21a6c9 100644 --- a/workflow/README.md +++ b/workflow/README.md @@ -78,6 +78,8 @@ result: | # optional; what text mode prints instead of t A step is a `command` or a `script`, never both. +A flag set to `false` in a step's `args` is left out of the command, the same as not naming it. So a step cannot turn off a flag that defaults to true, such as `static get-image`'s `attribution`; a script step can, by running `$MAPBOX_CLI static get-image ... --attribution=false`. + ### Versions A workflow's `version` is its own, not the CLI's: workflows are released on their own schedule. It is optional, and a workflow without one installs and runs the same. `list`, `show` and `install` report it. A release is tagged `workflow--`, such as `workflow-copy-style-0.1.0`, and installed with `--ref` set to that tag.