fix: correct --force flag logic in update and add --limit validation …#145
Open
emanmunir wants to merge 1 commit intoandrewyng:mainfrom
Open
fix: correct --force flag logic in update and add --limit validation …#145emanmunir wants to merge 1 commit intoandrewyng:mainfrom
emanmunir wants to merge 1 commit intoandrewyng:mainfrom
Conversation
…in search Two small bug fixes: 1. update.js: `opts.force || true` always evaluates to true, making the --force flag a no-op. `fetchAllRegistries` defaults its parameter to false, so passing `opts.force` (undefined when omitted) correctly falls back to cache-aware behavior while still honoring --force when the user explicitly sets it. 2. search.js: `parseInt(opts.limit, 10)` can produce NaN when a non-numeric value is passed (e.g. `--limit abc`). `Array.slice(0, NaN)` returns an empty array, silently hiding all results. Added `|| 20` fallback to match the declared default. Made-with: Cursor
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
2 participants
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.
Here is the filled-out PR description you can paste into the GitHub PR comment:
What
Two one-line bug fixes in CLI commands:
update.js— Replaceopts.force || truewithopts.forceso the--forceflag actually controls cache bypass.search.js— Add|| 20fallback toparseInt(opts.limit, 10)so a non-numeric--limitvalue doesn't silently return zero results.Why
update.js (line 34):
opts.force || trueis a logical short-circuit that always evaluates totrue. This meansfetchAllRegistriesis called withforce = trueon everychub update, bypassing the cache unconditionally — the--forceflag has no observable effect. With this fix, omitting--forcepassesundefined, which falls back to the function's default parameter (force = false), restoring cache-aware behavior.search.js (line 59):
parseInt("abc", 10)returnsNaN, andArray.slice(0, NaN)returns an empty array. A user passing--limit abc(or any non-numeric string) would silently get zero results with no error message. The|| 20fallback matches the default value already declared in the Commander.option()call.Testing
npm testpasses — all 166 passing tests remain passing; the 8 pre-existing failures are Windows path-separator issues unrelated to this changechub build content/ --validate-onlysucceeds (note:sample-content/was removed in an earlier commit; see CONTRIBUTING.md references removed sample-content directory #47)chub update— uses cached registry when fresh (no longer force-refreshes every time)chub update --force— bypasses cache as expectedchub search "stripe" --limit abc— falls back to 20 results instead of returning emptyNotes
sample-content/path in this PR template is stale (directory was deleted; tracked in CONTRIBUTING.md references removed sample-content directory #47). Usedcontent/for validation instead.