Skip to content

Commit 74bbef1

Browse files
Yash-ChindamYash-Chindam
committed
fix(presets): stop advertising a bare-id install as a guaranteed fix
Addresses review feedback on #4250. The per-dependency line labelled `specify extension add <id>` as "Fix with", while the closing note said an archive URL is required -- the remedy and the note contradicted each other, and for every extension motivating this feature the bare-id command is refused outright, since all three are listed only in the discovery-only community catalog. Label the remedy by the action it performs rather than asserting it fixes the problem: "Install with", "Reinstall with", "Enable with", and "Needs" for a version constraint, which was never a command in the first place. Replace the contradictory note with what actually happens. The discovery-only rejection prints the exact `--from <archive-url>` invocation to use, so the bare command is a signpost rather than a dead end, and saying so is both accurate and useful. Determining which catalog an extension came from would require a catalog fetch on an install path that touches no network, so the note stays unconditional and is now only emitted when a suggested command actually resolves through the catalogs. Assisted-by: Claude Code (model: Claude Opus 5, autonomous)
1 parent b61050e commit 74bbef1

2 files changed

Lines changed: 36 additions & 17 deletions

File tree

‎src/specify_cli/presets/_commands.py‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None:
5757

5858
console.print()
5959
console.print("[yellow]![/yellow] This preset depends on extensions that are not satisfied:")
60+
needs_catalog = False
6061
for dep in unmet:
62+
uses_catalog = False
6163
extension_id = _escape_markup(dep["id"])
6264
# The displayed id only needs Rich escaping, but a suggested command
6365
# has to survive Typer's parser: `^[a-z0-9-]+$` admits a leading
@@ -73,34 +75,41 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None:
7375
# downgrade, so do not promise that update will satisfy it.
7476
if reason == "missing":
7577
console.print(f" [yellow]{extension_id}[/yellow] is not installed")
76-
remedy = f"specify extension add {command_id}"
78+
label, remedy = "Install with", f"specify extension add {command_id}"
79+
uses_catalog = True
7780
elif reason == "corrupt":
7881
console.print(
7982
f" [yellow]{extension_id}[/yellow] has an unreadable "
8083
"registry entry"
8184
)
8285
# is_installed() still counts the key, so a plain add is refused.
86+
label = "Reinstall with"
8387
remedy = f"specify extension add {command_id} --force"
88+
uses_catalog = True
8489
elif reason == "stale":
8590
console.print(
8691
f" [yellow]{extension_id}[/yellow] is registered but its "
8792
"files are missing"
8893
)
94+
label = "Reinstall with"
8995
remedy = f"specify extension add {command_id} --force"
96+
uses_catalog = True
9097
elif reason == "disabled":
9198
console.print(f" [yellow]{extension_id}[/yellow] is installed but disabled")
92-
remedy = f"specify extension enable {command_id}"
99+
label, remedy = "Enable with", f"specify extension enable {command_id}"
93100
else:
94101
console.print(
95102
f" [yellow]{extension_id}[/yellow] "
96103
f"{_escape_markup(dep['installed'])} does not satisfy "
97104
f"{_escape_markup(dep['version'])}"
98105
)
106+
label = "Needs"
99107
remedy = (
100-
"install a release of "
101-
f"{command_id} satisfying {_escape_markup(dep['version'])}"
108+
f"a release of {command_id} satisfying "
109+
f"{_escape_markup(dep['version'])}"
102110
)
103-
console.print(f" Fix with: {remedy}")
111+
console.print(f" {label}: {remedy}")
112+
needs_catalog = needs_catalog or uses_catalog
104113
console.print()
105114
# The consequence differs by reason and must not be overstated. An
106115
# unavailable extension contributes nothing, so those features are simply
@@ -121,14 +130,19 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None:
121130
"[dim]Where only a version constraint is unmet the extension is "
122131
"still used, so it may not behave as the preset expects.[/dim]"
123132
)
124-
if any(dep["reason"] in ("missing", "corrupt", "stale") for dep in unmet):
133+
if needs_catalog:
125134
# `extension add <id>` resolves through the catalogs, and the default
126-
# community catalog is discovery-only, so installing by id alone is
127-
# rejected for anything listed only there. Say so once rather than
128-
# per dependency, since determining it would mean a catalog fetch.
135+
# community catalog is discovery-only, so installing by id is refused
136+
# for anything listed only there -- true of every extension motivating
137+
# this feature. Knowing which applies would mean a catalog fetch, and
138+
# this runs on an install path that touches no network, so describe
139+
# the outcome instead of asserting the command succeeds. The rejection
140+
# itself prints the exact --from form, so this is a signpost rather
141+
# than a dead end.
129142
console.print(
130-
"[dim]An extension listed only in a discovery-only catalog cannot be "
131-
"installed by id; pass --from <archive-url> with its release archive.[/dim]"
143+
"[dim]If an extension is listed only in a discovery-only catalog, "
144+
"that command is refused and prints the "
145+
"--from <archive-url> form to use instead.[/dim]"
132146
)
133147

134148

‎tests/test_presets.py‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,8 +1304,8 @@ def test_version_warning_does_not_promise_update_satisfies_constraint(self):
13041304
with console.capture() as capture:
13051305
_warn_unmet_extension_dependencies(manager, MagicMock())
13061306

1307-
output = strip_ansi(capture.get())
1308-
assert "install a release of speckit-inventory satisfying <2" in output
1307+
output = " ".join(strip_ansi(capture.get()).split())
1308+
assert "Needs: a release of speckit-inventory satisfying <2" in output
13091309
assert "specify extension update" not in output
13101310

13111311
def test_optional_dependency_is_never_reported(
@@ -1421,6 +1421,7 @@ def test_missing_and_stale_warnings_mention_discovery_only_catalogs(self):
14211421
output = strip_ansi(capture.get())
14221422
assert "discovery-only catalog" in output
14231423
assert "--from <archive-url>" in output
1424+
assert "Install with: specify extension add speckit-inventory" in output
14241425

14251426
@pytest.mark.parametrize(
14261427
"reason, extra",
@@ -1448,10 +1449,14 @@ def test_leading_hyphen_id_is_not_emitted_into_a_command(self, reason, extra):
14481449
with console.capture() as capture:
14491450
_warn_unmet_extension_dependencies(manager, MagicMock())
14501451

1451-
output = strip_ansi(capture.get())
1452-
remedy = next(
1453-
line for line in output.splitlines() if "Fix with:" in line
1452+
output = " ".join(strip_ansi(capture.get()).split())
1453+
# Isolate the remedy: the description line legitimately shows the raw
1454+
# id, escaped for display; only the copyable command must not carry it.
1455+
label = next(
1456+
lbl for lbl in ("Install with:", "Reinstall with:", "Enable with:", "Needs:")
1457+
if lbl in output
14541458
)
1459+
remedy = output.split(label, 1)[1].split("The preset is installed.")[0]
14551460
assert "--not-a-real-flag" not in remedy
14561461
assert "<extension-id>" in remedy
14571462

@@ -1516,7 +1521,7 @@ def test_corrupt_warning_suggests_forced_reinstall(self):
15161521

15171522
output = strip_ansi(capture.get())
15181523
assert "unreadable registry entry" in output
1519-
assert "specify extension add speckit-inventory --force" in output
1524+
assert "Reinstall with: specify extension add speckit-inventory --force" in output
15201525

15211526
def test_unreadable_registry_does_not_raise(
15221527
self, project_dir, temp_dir, valid_pack_data, monkeypatch

0 commit comments

Comments
 (0)