fix: make block_number optional + allow non-interactive pull all#16
Open
greatmerlin wants to merge 2 commits into
Open
fix: make block_number optional + allow non-interactive pull all#16greatmerlin wants to merge 2 commits into
pull all#16greatmerlin wants to merge 2 commits into
Conversation
The newest cards on the official site (e.g. OP16-063 Kuzan) have no Standard
block assigned yet, so the `div.block` element is empty. `fetch_block_number`
treated this as fatal (`bail!("card.block_number is empty!")`), which aborted
the entire `pull all` run mid-way.
Change `Card.block_number` to `Option<i32>` and have `fetch_block_number`
return `Ok(None)` when the block node is absent or contains no digits, rather
than bailing. Genuine parse errors (non-empty, non-numeric) still error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`pull all` was always interactive (inquire prompts for language, output dir, and images), which made it impossible to script or run in CI. Now, when an output directory is supplied with `-o/--output`, `pull all` skips every prompt and takes the language from `-l/--language` and the image choice from a new `-a/--with-images` flag (mirroring `pull cards`). With no `-o`, the original interactive prompts are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Hi @greatmerlin, It seems your fork is not rebased to the latest upstream commit. Part of your PR was already fixed by #10. |
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
Two changes, both prompted by
vega pull allaborting on the newest cards.1.
fix: don't abort the whole scrape when a card has no Standard blockThe newest cards on the official site (e.g. OP16-063 Kuzan) have no Standard block assigned yet, so the
dd>div.backCol>div.col2>div.blockelement is empty.fetch_block_numbertreated this as fatal:…which aborts the entire
pull allrun partway through.This changes
Card.block_numberfromi32toOption<i32>, andfetch_block_numbernow returnsOk(None)when the block node is absent or contains no digits, instead ofbail!-ing. Genuine parse errors (non-empty, non-numeric) still error.2.
feat: non-interactivepull allvia-oand--with-imagespull allwas always interactive (inquire prompts for language, output dir, images), so it couldn't be scripted or run in CI. Now, when-o/--outputis supplied,pull allskips the prompts and takes the language from-l/--languageand the image choice from a new-a/--with-imagesflag (mirroringpull cards). With no-o, the interactive flow is unchanged.Testing
cargo build --releaseis clean.vega pull -l english -o data all -ascrapes all 53 packs + 4571 images with no crash; OP16-063 is captured withblock_number: null.