Fix media folder handling in CLI build command#20
Conversation
Convert CLI string path to Path object and pass it to AnkiBuilder constructor to enable automatic media discovery for non-batch builds. Also update internal variable naming from final_media_dir to media_folder for clarity.
mrMaxwellTheCat
deleted the
analiza-el-archivo-roadmap-md-para-identificar-l-1770999499358
branch
There was a problem hiding this comment.
Pull request overview
This pull request fixes a bug where the media folder path was not being passed to the AnkiBuilder constructor in the simple build command, preventing automatic media discovery for non-batch builds. The fix converts the CLI string path to a Path object and passes it to AnkiBuilder. The PR also improves variable naming for clarity by renaming final_media_dir to media_folder.
Changes:
- Fixed media folder handling in CLI
buildcommand by passing the media_folder parameter to AnkiBuilder constructor - Improved variable naming from
final_media_dirtomedia_folderfor consistency - Updated ROADMAP.md to mark the bug fix as completed
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/anki_yaml_tool/cli.py | Converts CLI media_dir string to Path object, renames variable to media_folder, and passes it to AnkiBuilder to enable media discovery |
| ROADMAP.md | Marks the media_folder fix as completed in the critical bugs section |
| .gitignore | Adds session-id files to ignore list (unrelated to main fix) |
Comments suppressed due to low confidence (1)
src/anki_yaml_tool/cli.py:295
- Variable has_errors is not used.
has_errors = True
| click.echo(f"Building deck '{final_deck_name}'...") | ||
| log.info("Output file: %s", output) | ||
| builder = AnkiBuilder(final_deck_name, model_configs) | ||
| builder = AnkiBuilder(final_deck_name, model_configs, media_folder) |
There was a problem hiding this comment.
The media_folder parameter should be passed as a named argument for consistency with how it's used in _batch_build_separate (line 1108) and to avoid breaking existing tests that check positional arguments. Change to: builder = AnkiBuilder(final_deck_name, model_configs, media_folder=media_folder)
| builder = AnkiBuilder(final_deck_name, model_configs, media_folder) | |
| builder = AnkiBuilder( | |
| final_deck_name, | |
| model_configs, | |
| media_folder=media_folder, | |
| ) |
Convert CLI string path to a Path object and pass it to the AnkiBuilder constructor to enable automatic media discovery for non-batch builds. Update internal variable naming for clarity.