refactor: move preset command handlers to presets/_commands.py (PR-6/8)#2826
Open
darion-yaphet wants to merge 3 commits into
Open
refactor: move preset command handlers to presets/_commands.py (PR-6/8)#2826darion-yaphet wants to merge 3 commits into
darion-yaphet wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the CLI by moving the specify preset ... Typer command groups and handlers out of src/specify_cli/__init__.py into a co-located src/specify_cli/presets/_commands.py, continuing the effort to slim __init__.py into a thin registration layer while keeping the public specify_cli.presets import surface stable.
Changes:
- Introduces
presets/_commands.pycontaining thepreset/preset catalogTyper apps and all associated handlers, plus aregister(app)entry point. - Repackages
presets.pyinto thepresets/package and rebases internal imports accordingly. - Updates
specify_cli/__init__.pyto register preset commands viapresets._commands.register()and keeps_locate_bundled_presetre-exported.
Show a summary per file
| File | Description |
|---|---|
| src/specify_cli/presets/_commands.py | New home for preset CLI Typer apps + command handlers, plus register(app) wiring. |
| src/specify_cli/presets/init.py | Rebased imports to account for presets/ becoming a package. |
| src/specify_cli/init.py | Removes inlined preset command handlers and registers them via presets/_commands.py. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 4
35a36e1 to
566974e
Compare
Pure structural move to mirror integrations/. presets.py becomes presets/__init__.py with relative imports rebased one level deeper. No behavior change; public import surface (from .presets import ...) preserved. Prepares for co-locating preset command handlers in PR-6/8.
Cut the preset_app / preset_catalog_app Typer groups and all 12 command handlers out of __init__.py into presets/_commands.py, exposing register(app) — mirrors the integration co-location from PR-5. __init__.py now registers via _register_preset_cmds(app), dropping ~620 lines (3282 -> 2663). Handlers lazy-import root helpers (_require_specify_project, get_speckit_version, _locate_bundled_preset, _display_project_path) via 'from .. import' so test monkeypatching of specify_cli.<helper> keeps working. _locate_bundled_preset kept as an explicit re-export in __init__.py for that resolution path. CLI surface and public imports unchanged. Full suite: 3162 passed, 40 skipped.
Preset URL installs already rejected non-HTTPS source URLs, but the authenticated opener follows redirects. Validate the final response URL before writing the ZIP, preserve GitHub release asset URL resolution after the preset command module split, stream the response to disk, and keep catalog config serialization on safe YAML output. Constraint: open_url follows redirects, so source URL validation alone does not constrain the downloaded target Rejected: Keep response.read() for simplicity | large preset downloads should not be buffered entirely in memory Confidence: high Scope-risk: narrow Directive: Keep preset URL policy aligned with workflow installer redirect validation Tested: uvx ruff check src/specify_cli/__init__.py src/specify_cli/presets/__init__.py src/specify_cli/presets/_commands.py tests/test_presets.py Tested: uv run pytest tests/test_presets.py -q Not-tested: Real network redirect integration against a live HTTP server Co-authored-by: OmX <omx@oh-my-codex.dev>
566974e to
844e316
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR-6 of the
__init__.pysplit (continues PR-5's integration co-location). Moves thepresetcommand group out ofsrc/specify_cli/__init__.pyinto a co-locatedpresets/_commands.py, trimming__init__.pytoward a thin registration layer.Two commits:
presets.py→presets/package — pure structural move so command handlers can live beside the preset domain logic (mirrorsintegrations/).presets.pybecomespresets/__init__.py; package-relative imports rebased one level deeper. Public import surface (from specify_cli.presets import ...) unchanged.presets/_commands.py— thepreset_app/preset_catalog_appTyper groups and all 12 handlers (list,add,remove,search,resolve,info,set-priority,enable,disable,catalog list/add/remove) move out;__init__.pyregisters them via_register_preset_cmds(app).__init__.pydrops ~620 lines in the preset region.Design notes
_require_specify_project,get_speckit_version,_locate_bundled_preset,_display_project_path) viafrom .. importinside each function, so test monkeypatching ofspecify_cli.<helper>keeps working — same pattern PR-5 used._locate_bundled_presetkept as an explicit re-export in__init__.pyfor that resolution path.Test plan
python -c "import specify_cli"— OKspecify preset --help— full command surface intactuvx ruff check src/— All checks passed