Skip to content

Commit aaec670

Browse files
committed
fix: address sixteenth round of Copilot PR review feedback
- Remove dead has_composition branch in resolve_content: after the top-replace early return, the top layer is guaranteed non-replace so the any() check was always true - Fix CLI composition chain base labeling: compute effective base index using same logic as resolve_content (last consecutive replace from bottom before first non-replace) instead of always labeling i==0 - Support extension manifest file paths in collect_all_layers: when convention lookup fails for commands, parse extension.yml to find the actual file path, enabling composition onto extension commands that use non-convention filenames
1 parent 3a62a64 commit aaec670

File tree

2 files changed

+31
-12
lines changed

2 files changed

+31
-12
lines changed

src/specify_cli/__init__.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2612,12 +2612,20 @@ def preset_resolve(
26122612
if has_composition:
26132613
console.print(" [dim]Final output is composed from multiple preset layers; the path above is the highest-priority contributing layer.[/dim]")
26142614
console.print("\n [bold]Composition chain:[/bold]")
2615-
for i, layer in enumerate(reversed(layers)):
2615+
# Compute the effective base: the last consecutive replace layer
2616+
# from the bottom before the first non-replace (same logic as
2617+
# PresetResolver.resolve_content).
2618+
reversed_display = list(reversed(layers))
2619+
effective_base_idx = 0
2620+
for idx, lyr in enumerate(reversed_display):
2621+
if lyr["strategy"] == "replace":
2622+
effective_base_idx = idx
2623+
else:
2624+
break
2625+
for i, layer in enumerate(reversed_display):
26162626
strategy_label = layer["strategy"]
2617-
if strategy_label == "replace":
2618-
# Only mark the effective base (the one composition builds on)
2619-
if i == 0:
2620-
strategy_label = "base"
2627+
if strategy_label == "replace" and i == effective_base_idx:
2628+
strategy_label = "base"
26212629
console.print(f" {i + 1}. [{strategy_label}] {layer['source']}{layer['path']}")
26222630
else:
26232631
# No layers found — fall back to resolve_with_source for non-composition cases

src/specify_cli/presets.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2320,7 +2320,25 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
23202320
ext_dir = self.extensions_dir / ext_id
23212321
if not ext_dir.is_dir():
23222322
continue
2323+
# Try convention-based lookup first
23232324
candidate = _find_in_subdirs(ext_dir)
2325+
# If not found and this is a command, check extension manifest
2326+
if candidate is None and template_type == "command":
2327+
ext_manifest_path = ext_dir / "extension.yml"
2328+
if ext_manifest_path.exists():
2329+
try:
2330+
from .extensions import ExtensionManifest
2331+
ext_manifest = ExtensionManifest(ext_manifest_path)
2332+
for cmd in ext_manifest.commands:
2333+
if cmd.get("name") == template_name:
2334+
cmd_file = cmd.get("file")
2335+
if cmd_file:
2336+
c = ext_dir / cmd_file
2337+
if c.exists():
2338+
candidate = c
2339+
break
2340+
except Exception:
2341+
pass
23242342
if candidate:
23252343
if ext_meta:
23262344
version = ext_meta.get("version", "?")
@@ -2395,13 +2413,6 @@ def resolve_content(
23952413
if layers[0]["strategy"] == "replace":
23962414
return layers[0]["path"].read_text(encoding="utf-8")
23972415

2398-
# Check if any layer uses a non-replace strategy
2399-
has_composition = any(layer["strategy"] != "replace" for layer in layers)
2400-
2401-
if not has_composition:
2402-
# Pure replacement — just read the highest-priority file
2403-
return layers[0]["path"].read_text(encoding="utf-8")
2404-
24052416
# Composition: build content bottom-up (lowest priority first)
24062417
# Start from the lowest-priority "replace" layer as the base,
24072418
# then apply composition layers on top.

0 commit comments

Comments
 (0)