Skip to content

Commit c2012d9

Browse files
committed
Verify generated Carapace specs against the real carapace-spec engine
1 parent c17f397 commit c2012d9

3 files changed

Lines changed: 232 additions & 41 deletions

File tree

changelog.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
> [!WARNING]
66
> This version is **not released yet** and is under active development.
77
8-
- Fix Carapace dynamic completion for subcommand parameters, which resolved against the root command and returned the wrong candidates.
8+
- Fix Carapace dynamic completion: an option's value or a subcommand argument with a custom `shell_complete` now resolves through the generated spec, instead of returning empty or root-level candidates.
9+
- Verify generated Carapace specs against the real `carapace-spec` engine in the test suite.
910

1011
## [`8.1.4` (2026-06-27)](https://github.com/kdeldycke/click-extra/compare/v8.1.3...v8.1.4)
1112

click_extra/carapace.py

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ def _env_var(prog_name: str) -> str:
171171
return f"_{prog_name}_COMPLETE".replace("-", "_").upper()
172172

173173

174-
def _dynamic_action(command_path: tuple[str, ...]) -> str:
174+
def _dynamic_action(command_path: tuple[str, ...], option: str | None = None) -> str:
175175
"""A Carapace shell-macro action that calls back into the CLI for completion.
176176
177177
Carapace's default ``$(...)`` macro runs ``sh -c '<script>' -- <words>``,
@@ -181,27 +181,37 @@ def _dynamic_action(command_path: tuple[str, ...]) -> str:
181181
shell), which prints exactly that. Carapace prefix-filters the result, so the
182182
callback returns the full candidate set.
183183
184-
``command_path`` is the chain of command names from the root program down to
185-
the command owning the completed parameter, like ``("weather", "forecast")``.
186-
It is baked into ``COMP_WORDS`` so :class:`CarapaceComplete` can rebuild the
187-
command line Click needs to resolve the subcommand.
188-
189-
.. note::
190-
Baking the whole path is required, not cosmetic. Carapace's ``traverse``
191-
descends into each subcommand with only the remaining words, stripping the
192-
parent command names from the ``c.Args`` it hands the macro. So ``$*``
193-
carries only the leaf command's own words: for ``weather forecast --city``
194-
Carapace passes just ``--city``. Without the baked ``weather forecast``
195-
prefix, the callback would reconstruct ``weather --city`` and resolve
196-
against the root command, completing the wrong thing. The env var and the
197-
invoked binary stay rooted at ``command_path[0]``, the executable Carapace
198-
dispatches and the name Click derives its completion variable from.
184+
``COMP_WORDS`` is rebuilt so :class:`CarapaceComplete` hands Click the command
185+
line it needs to resolve the completed parameter. Two pieces must be baked in,
186+
because Carapace withholds them from ``$*``:
187+
188+
- ``command_path``: the chain of command names from the root program down to
189+
the command owning the parameter, like ``("weather", "forecast")``.
190+
Carapace's ``traverse`` descends into each subcommand with only the remaining
191+
words, so ``$*`` never carries the parent command names. Without the baked
192+
prefix the callback would reconstruct ``weather --city`` and resolve against
193+
the root command.
194+
195+
- ``option``: the spelling of the flag whose value is being completed, for an
196+
option (not an argument). Carapace routes a flag's value completion to this
197+
per-flag action and keeps the flag in its own ``inFlag`` state rather than
198+
``c.Args``, so ``$*`` does not contain it; Click would not know which value
199+
to resolve. It is appended after ``$*`` so it stays the trailing token Click
200+
reads as the option awaiting a value.
201+
202+
So a subcommand option ``--city`` yields ``COMP_WORDS=weather forecast $*
203+
--city``, while a subcommand argument yields ``COMP_WORDS=weather forecast
204+
$*``. The env var and the invoked binary stay rooted at ``command_path[0]``,
205+
the executable Carapace dispatches and the name Click derives its completion
206+
variable from.
199207
"""
200208
root = command_path[0]
201-
words = " ".join(command_path)
209+
comp_words = " ".join(command_path) + " $*"
210+
if option:
211+
comp_words += f" {option}"
202212
return (
203213
f'$(env "{_env_var(root)}=carapace_complete" '
204-
f'"COMP_WORDS={words} $*" {root} 2>/dev/null)'
214+
f'"COMP_WORDS={comp_words}" {root} 2>/dev/null)'
205215
)
206216

207217

@@ -234,21 +244,26 @@ def _overrides_shell_complete(param_type: click.ParamType) -> bool:
234244
return type(param_type).shell_complete is not click.ParamType.shell_complete
235245

236246

237-
def _param_action(param: Parameter, command_path: tuple[str, ...]) -> list[str]:
247+
def _param_action(
248+
param: Parameter, command_path: tuple[str, ...], option: str | None = None
249+
) -> list[str]:
238250
"""Resolve the Carapace completion action for one parameter.
239251
240252
An explicit ``shell_complete=`` callback always routes to the dynamic macro;
241253
otherwise a statically-knowable type is inlined; otherwise a type that
242254
overrides ``shell_complete`` falls back to the dynamic macro. Anything else
243-
yields an empty action (no completion offered).
255+
yields an empty action (no completion offered). ``option`` carries the flag
256+
spelling when the parameter is an option, so the dynamic macro can name the
257+
flag whose value is being completed (see :func:`_dynamic_action`); arguments
258+
leave it ``None``.
244259
"""
245260
if getattr(param, "_custom_shell_complete", None) is not None:
246-
return [_dynamic_action(command_path)]
261+
return [_dynamic_action(command_path, option)]
247262
static = _static_action(param.type)
248263
if static is not None:
249264
return static
250265
if _overrides_shell_complete(param.type):
251-
return [_dynamic_action(command_path)]
266+
return [_dynamic_action(command_path, option)]
252267
return []
253268

254269

@@ -398,8 +413,9 @@ def _add_option(
398413
A boolean flag with a secondary spelling (``--foo`` / ``--no-foo``) is split
399414
into two independent Carapace flags, since the spec has no negation primitive.
400415
Only value-taking options contribute a ``completion.flag`` action. Dynamic
401-
actions reference ``command_path``, the chain of command names down to this
402-
command (see :func:`_dynamic_action`).
416+
actions reference ``command_path`` (the chain of command names down to this
417+
command) and the option's own spelling, both baked into the callback so Click
418+
can resolve the right flag's value (see :func:`_dynamic_action`).
403419
"""
404420
flags = node.persistentflags if persistent else node.flags
405421
description = _clean_description(getattr(param, "help", None))
@@ -418,7 +434,8 @@ def _add_option(
418434
)
419435

420436
if value:
421-
action = _param_action(param, command_path)
437+
short, long = short_long_opts(param.opts)
438+
action = _param_action(param, command_path, option=long or short)
422439
if action:
423440
node.completion.flag[_flag_name(param.opts)] = action
424441

tests/test_carapace.py

Lines changed: 188 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,18 @@
1717
from __future__ import annotations
1818

1919
import json
20+
import os
21+
import platform
22+
import stat
23+
import subprocess
24+
import sys
25+
import tarfile
2026
from pathlib import Path
2127

2228
import click
2329
import cloup
2430
import pytest
31+
import requests
2532
import yaml
2633
from click.shell_completion import CompletionItem, get_completion_class
2734
from cloup.constraints import mutually_exclusive
@@ -218,10 +225,12 @@ def test_dynamic_action_emitted(flag_name):
218225
macro = action[0]
219226
assert macro.startswith("$(")
220227
assert "_WEATHER_COMPLETE=carapace_complete" in macro
221-
# The macro bakes the full command path (root + subcommand), not just the
222-
# root: Carapace strips parent command names from the words it forwards to
223-
# the macro, so the path must be restored here for Click to resolve forecast.
224-
assert "COMP_WORDS=weather forecast $*" in macro
228+
# The macro bakes the full command path (root + subcommand) AND the option
229+
# spelling. Carapace forwards neither to the macro: it strips parent command
230+
# names from the words, and routes a flag's value completion to this per-flag
231+
# action without putting the flag in those words. Both are restored so Click
232+
# resolves the right option's value under the right subcommand.
233+
assert f"COMP_WORDS=weather forecast $* --{flag_name}" in macro
225234

226235

227236
# -- persistentflags ----------------------------------------------------------
@@ -323,21 +332,26 @@ def test_complete_reuses_click_machinery():
323332

324333

325334
def test_subcommand_dynamic_completion_resolves_end_to_end(monkeypatch):
326-
"""The COMP_WORDS a subcommand's generated macro builds must let the backend
327-
resolve that subcommand's callback, not the root command.
328-
329-
Carapace strips parent command names from the words it forwards to the macro,
330-
so for ``weather forecast --city`` it passes only ``--city``. The macro
331-
restores the baked path; rebuilding COMP_WORDS the same way and feeding it
332-
back must yield the forecast callback's values. Pinning only the root name
333-
(the original behavior) would reconstruct ``weather --city``, resolve it
334-
against the root, and complete the subcommand list instead.
335+
"""The COMP_WORDS a subcommand flag's generated macro builds must let the
336+
backend resolve that option's value, not the root command.
337+
338+
When completing a subcommand flag's value, Carapace forwards an empty word
339+
list to the macro: it strips the parent command names, and keeps the flag
340+
being completed in its own state rather than passing it through. So the macro
341+
bakes both the command path and the flag spelling, and ``$*`` expands to
342+
nothing. This reconstructs the resulting COMP_WORDS from the generated macro
343+
and confirms the backend resolves the forecast callback. The original
344+
root-only, flag-less macro would have reconstructed bare ``weather`` and
345+
completed the subcommand list instead.
335346
"""
336347
macro = FORECAST["completion"]["flag"]["city"][0]
337-
baked_path = macro.split("COMP_WORDS=", 1)[1].split(" $*", 1)[0]
338-
monkeypatch.setenv("COMP_WORDS", f"{baked_path} --city")
348+
# The macro's COMP_WORDS template, with $* expanded to the empty word list
349+
# Carapace passes for a flag value.
350+
template = macro.split("COMP_WORDS=", 1)[1].split('"', 1)[0]
351+
monkeypatch.setenv("COMP_WORDS", template.replace("$*", ""))
339352
comp = CarapaceComplete(weather, {}, "weather", "_WEATHER_COMPLETE")
340353
args, incomplete = comp.get_completion_args()
354+
assert args == ["forecast", "--city"]
341355
values = {item.value for item in comp.get_completions(args, incomplete)}
342356
assert values == {"paris", "oslo"}
343357

@@ -416,3 +430,162 @@ def test_wrap_install_requires_carapace(runner, greet_script):
416430
result = runner.invoke(demo, ["wrap", "--install", greet_script], color=False)
417431
assert result.exit_code == 2
418432
assert "--install requires --carapace" in result.output
433+
434+
435+
# -- Real carapace-spec engine (integration) ----------------------------------
436+
437+
# Pinned carapace-spec release exercised by the integration test below.
438+
CARAPACE_SPEC_VERSION = "1.7.0"
439+
440+
# platform.machine() (lower-cased) -> carapace-spec release arch token.
441+
CARAPACE_ARCH = {
442+
"aarch64": "arm64",
443+
"amd64": "amd64",
444+
"arm64": "arm64",
445+
"x86_64": "amd64",
446+
}
447+
448+
# A self-contained weather CLI exercising each dynamic shell_complete shape: a
449+
# root-level option, a subcommand option, and a subcommand argument. Invoked with
450+
# --dump-carapace-spec it prints its own spec; otherwise it answers the completion
451+
# the spec's dynamic macros call back into.
452+
CARAPACE_RUNNER = '''\
453+
import sys
454+
455+
import click
456+
import cloup
457+
from click.shell_completion import CompletionItem
458+
459+
import click_extra # noqa: F401 Registers the carapace completion backend.
460+
461+
462+
def complete_region(ctx, param, incomplete):
463+
return [CompletionItem("north"), CompletionItem("south")]
464+
465+
466+
def complete_city(ctx, param, incomplete):
467+
return [CompletionItem("paris", help="France"), CompletionItem("oslo")]
468+
469+
470+
def complete_zone(ctx, param, incomplete):
471+
return [CompletionItem("alpha"), CompletionItem("bravo")]
472+
473+
474+
@cloup.group()
475+
@click.option("--region", shell_complete=complete_region, help="Region.")
476+
def weather(region):
477+
"""Weather tools."""
478+
479+
480+
@weather.command()
481+
@click.option("--city", shell_complete=complete_city, help="City.")
482+
@click.argument("zone", shell_complete=complete_zone)
483+
def forecast(city, zone):
484+
"""Forecast for a zone."""
485+
486+
487+
if __name__ == "__main__":
488+
if "--dump-carapace-spec" in sys.argv:
489+
from click_extra.carapace import dump_carapace_spec
490+
491+
sys.stdout.write(dump_carapace_spec(weather, prog_name="weather"))
492+
else:
493+
weather(prog_name="weather")
494+
'''
495+
496+
497+
@pytest.mark.network
498+
@pytest.mark.once
499+
def test_spec_drives_real_carapace_engine(tmp_path):
500+
"""Feed a generated spec to the real carapace-spec binary, end-to-end.
501+
502+
The tests above simulate Carapace at the CarapaceComplete level. This one
503+
downloads a pinned carapace-spec release and drives it through each dynamic
504+
shape: a root option, a subcommand option, and a subcommand argument. It
505+
guards the contract between the emitted macros and Carapace's real
506+
traverse/exec/parse behavior, which the simulation cannot see: Carapace strips
507+
parent command names from the words it forwards, and withholds the flag being
508+
completed entirely, so the macros must bake both back in.
509+
510+
.. attention::
511+
Downloads the carapace-spec binary from GitHub and executes it, hence the
512+
``network`` mark (and ``once``, to run on a single CI runner). The dynamic
513+
macro shells out, so the test only runs on POSIX platforms.
514+
"""
515+
system = platform.system().lower()
516+
arch = CARAPACE_ARCH.get(platform.machine().lower())
517+
if system not in ("linux", "darwin"):
518+
pytest.skip(f"dynamic macro is POSIX-shell only, not {system}")
519+
if arch is None:
520+
pytest.skip(f"no carapace-spec asset for {platform.machine()}")
521+
522+
# Download the pinned release archive from GitHub.
523+
asset = f"carapace-spec_{CARAPACE_SPEC_VERSION}_{system}_{arch}.tar.gz"
524+
url = (
525+
"https://github.com/carapace-sh/carapace-spec/releases/download/"
526+
f"v{CARAPACE_SPEC_VERSION}/{asset}"
527+
)
528+
with requests.get(url, timeout=60) as response:
529+
assert response.ok, f"failed to download {url}"
530+
archive_bytes = response.content
531+
532+
archive = tmp_path / asset
533+
archive.write_bytes(archive_bytes)
534+
535+
# Pull just the carapace-spec binary out of the archive by name. Reading the
536+
# member by hand (rather than extractall) sidesteps path-traversal and symlink
537+
# members and the cross-version tar extraction-filter churn.
538+
with tarfile.open(archive, "r:gz") as tar:
539+
member = next(
540+
(
541+
m
542+
for m in tar.getmembers()
543+
if m.isfile() and Path(m.name).name == "carapace-spec"
544+
),
545+
None,
546+
)
547+
assert member is not None, tar.getnames()
548+
payload = tar.extractfile(member)
549+
assert payload is not None
550+
binary = tmp_path / "carapace-spec"
551+
binary.write_bytes(payload.read())
552+
binary.chmod(binary.stat().st_mode | stat.S_IXUSR)
553+
554+
# Write the CLI and generate its spec through the script's own dump mode.
555+
script = tmp_path / "weather_cli.py"
556+
script.write_text(CARAPACE_RUNNER)
557+
spec = subprocess.run(
558+
[sys.executable, str(script), "--dump-carapace-spec"],
559+
capture_output=True,
560+
text=True,
561+
check=True,
562+
).stdout
563+
spec_file = tmp_path / "weather.yaml"
564+
spec_file.write_text(spec)
565+
566+
# The dynamic macro execs the program by bare name, so a `weather` shim that
567+
# runs the script under this interpreter must be discoverable on PATH.
568+
bindir = tmp_path / "bin"
569+
bindir.mkdir()
570+
shim = bindir / "weather"
571+
shim.write_text(f'#!/bin/sh\nexec "{sys.executable}" "{script}" "$@"\n')
572+
shim.chmod(0o755)
573+
env = {**os.environ, "PATH": f"{bindir}{os.pathsep}{os.environ['PATH']}"}
574+
575+
def complete(*words):
576+
"""Ask the real engine to complete `weather <words> <TAB>`."""
577+
result = subprocess.run(
578+
[str(binary), str(spec_file), "export", "weather", *words],
579+
capture_output=True,
580+
text=True,
581+
env=env,
582+
)
583+
assert result.returncode == 0, result.stderr
584+
return {value["value"] for value in json.loads(result.stdout)["values"]}
585+
586+
# Each dynamic shape resolves through the real engine.
587+
assert complete("--region", "") == {"north", "south"}
588+
assert complete("forecast", "--city", "") == {"paris", "oslo"}
589+
assert complete("forecast", "") == {"alpha", "bravo"}
590+
# Carapace prefix-filters the full candidate set the callback returns.
591+
assert complete("forecast", "--city", "p") == {"paris"}

0 commit comments

Comments
 (0)