fix(core): skip double-filtering when options is a function#489
fix(core): skip double-filtering when options is a function#489brick-pixel wants to merge 3 commits intobombshell-dev:mainfrom
Conversation
When `options` is provided as a function, the caller manages their own filtering (e.g. using fuzzysort). The autocomplete prompt was applying its default filter on top of the already-filtered results, silently dropping valid matches. Now, when `options` is a function and no custom `filter` was provided, the returned options are used as-is without additional filtering. If both `options` (function) and `filter` are provided, the filter is still applied as before, giving full control to the caller. Fixes bombshell-dev#488
🦋 Changeset detectedLatest commit: c900cf7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
for some context (cc @dreyfus92 too), we (or at least i) were aware of this. it is not a bug, it was an active decision that if you want to do your own filtering, you need to pass a noop filter until we get around to deciding how to solve it in a better way. so we're just now at the point of "deciding how to solve it in a better way". just wanted to make that clear so we're aware this is not a bug we're fixing. it is a behavioural change and a not all custom options functions do their own filtering, which is why we didn't simply turn it off like in this PR. i feel like it is a rare case, though, to have the need for an options function and not do your own filtering. so i think i am happy to change this. just make sure you update the PR title etc to |
43081j
left a comment
43081j
left a comment
There was a problem hiding this comment.
ah lastly can you add tests for the new branches?
- setting an options function doesn't apply the default filter
- setting a filter and an options function works (i.e. both run)
|
Thanks for the context! Updated the PR:
Makes sense that not all custom options functions do their own filtering. With this change, |
|
Added the tests — one for options function without filter (default filter skipped) and one with both options function and custom filter (filter still runs). All 98 core tests passing. |
| }); | ||
|
|
||
| test('options function skips default filter when no custom filter provided', () => { | ||
| const optionsFn = function (this: any) { |
There was a problem hiding this comment.
you don't need any in this repo as we always have a type. this one is an AutocompletePrompt
| input.emit('keypress', 'b', { name: 'b' }); | ||
|
|
||
| // Should use the options function result as-is, no additional filtering | ||
| expect(instance.filteredOptions).toEqual([ |
There was a problem hiding this comment.
wouldn't this pass if the default filter was being applied too?
you should probably make your options function do some unusual filtering. e.g. filter to a constant result
🤖 Automated account detected@brick-pixel has been flagged as a likely automated account. Classification:
Analyzed 61 public events via voight-kampff-test |
2 participants
What
When
optionsis provided as a function, the autocomplete prompt no longer applies the default filter on top of the function's already-filtered results.Why
As reported in #488, when
optionsis a function (e.g. using fuzzysort for custom ranking), clack's default substring filter runs on the returned list, silently dropping valid matches. This forces users to addfilter: () => trueas a workaround.Root cause: In
#onUserInputChanged,this.#filterFnis always applied to the result ofthis.options, regardless of whetheroptionsis a static array or a dynamic function.Changes
File:
packages/core/src/prompts/autocomplete.ts#hasCustomFilterfield to track whether a customfilterwas providedoptionsis a function and no customfilterwas provided, the returned options are used as-isoptions(function) andfilterare provided, the filter is still applied as beforeBehavior Matrix
optionsfilterVerification
Fixes #488