-
Notifications
You must be signed in to change notification settings - Fork 154
Fix #1019: Improve compile target specification #1020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,20 @@ | |
| # Valid target values (internal canonical form) | ||
| TargetType = Literal["vscode", "claude", "cursor", "opencode", "codex", "gemini", "all", "minimal"] | ||
|
|
||
| # Compiler families used inside a multi-target frozenset. Narrower than | ||
| # TargetType because the families are produced by _resolve_compile_target() | ||
| # (in the compile CLI) from CLI-validated target names. | ||
| CompileFamily = Literal["agents", "claude", "gemini"] | ||
|
|
||
| # Compile target: either a single TargetType string or a frozenset of compiler | ||
| # families ({"agents", "claude", "gemini"}) for multi-target lists. | ||
| CompileTargetType = Union[TargetType, frozenset[CompileFamily]] | ||
|
|
||
|
Comment on lines
+37
to
+40
|
||
| # Detection reason returned by detect_target() when no integration folder is | ||
| # present. Exported as a constant so consumers can compare with equality | ||
| # instead of substring matching. | ||
| REASON_NO_TARGET_FOLDER = "no target folder found" | ||
|
|
||
| # User-facing target values (includes aliases accepted by CLI) | ||
| UserTargetType = Literal["copilot", "vscode", "agents", "claude", "cursor", "opencode", "codex", "gemini", "all", "minimal"] | ||
|
|
||
|
|
@@ -120,45 +134,54 @@ def detect_target( | |
| elif gemini_exists: | ||
| return "gemini", "detected .gemini/ folder" | ||
| else: | ||
| return "minimal", "no target folder found" | ||
| return "minimal", REASON_NO_TARGET_FOLDER | ||
|
|
||
|
|
||
| def should_compile_agents_md(target: TargetType) -> bool: | ||
| def should_compile_agents_md(target: CompileTargetType) -> bool: | ||
| """Check if AGENTS.md should be compiled. | ||
|
|
||
| AGENTS.md is generated for vscode, codex, gemini, all, and minimal | ||
| targets. Gemini needs it because GEMINI.md imports AGENTS.md. | ||
|
|
||
| Args: | ||
| target: The detected or configured target | ||
|
|
||
| target: The detected or configured target. May be a string or a | ||
| frozenset of compiler families for multi-target lists. | ||
|
|
||
| Returns: | ||
| bool: True if AGENTS.md should be generated | ||
| """ | ||
| if isinstance(target, frozenset): | ||
| return "agents" in target or "gemini" in target | ||
| return target in ("vscode", "opencode", "codex", "gemini", "all", "minimal") | ||
|
|
||
|
|
||
| def should_compile_claude_md(target: TargetType) -> bool: | ||
| def should_compile_claude_md(target: CompileTargetType) -> bool: | ||
| """Check if CLAUDE.md should be compiled. | ||
|
|
||
| Args: | ||
| target: The detected or configured target | ||
| target: The detected or configured target. May be a string or a | ||
| frozenset of compiler families for multi-target lists. | ||
|
|
||
| Returns: | ||
| bool: True if CLAUDE.md should be generated | ||
| """ | ||
| if isinstance(target, frozenset): | ||
| return "claude" in target | ||
| return target in ("claude", "all") | ||
|
|
||
|
|
||
| def should_compile_gemini_md(target: TargetType) -> bool: | ||
| def should_compile_gemini_md(target: CompileTargetType) -> bool: | ||
| """Check if GEMINI.md should be compiled. | ||
|
|
||
| Args: | ||
| target: The detected or configured target | ||
| target: The detected or configured target. May be a string or a | ||
| frozenset of compiler families for multi-target lists. | ||
|
|
||
| Returns: | ||
| bool: True if GEMINI.md should be generated | ||
| """ | ||
| if isinstance(target, frozenset): | ||
| return "gemini" in target | ||
| return target in ("gemini", "all") | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detect_target()is typed to acceptOptional[str]forconfig_target, but herecompile_config_targetcan be afrozenset(from_resolve_compile_target()whenapm.ymlspecifies a multi-target list and the user also passed a single-string--target). It happens to work today, but it's an implicit type contract violation and makes the control flow harder to reason about. Consider passingconfig_target=compile_config_targetonly when it's a string (otherwiseNone).