Skip to content

Refactor sequence matching to accept item rather than word array#8866

Merged
donkawechico merged 3 commits intodevfrom
refactor-sequence-matching
Apr 14, 2021
Merged

Refactor sequence matching to accept item rather than word array#8866
donkawechico merged 3 commits intodevfrom
refactor-sequence-matching

Conversation

@donkawechico
Copy link
Copy Markdown
Contributor

@donkawechico donkawechico commented Apr 8, 2021

Proposed change

Preparation for quick-bar letter highlighting (Draft PR here: #8868). Changes the fuzzyFilterSort method to accept the whole item rather than an array of filter words.

This has a few benefits:

  1. Allows items to add an arbitrary number of "words" to represent it during filtration rather than just two.
  2. Makes it so the sorting method doesn't need to know about "altText" or "filterText".
  3. The sorter needs access to the item so that it can give the item a text decoration (for eventual letter highlighting). Without that, we'd have to loop over all the items a second time and apply the decoration algo.
  4. Might help resolve Quickbar doesn't show options in other language #8810

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@donkawechico donkawechico force-pushed the refactor-sequence-matching branch 2 times, most recently from c2a001c to a2ceb7f Compare April 8, 2021 22:29
@donkawechico donkawechico force-pushed the refactor-sequence-matching branch from a2ceb7f to 64c008f Compare April 8, 2021 22:29
Comment thread src/common/string/filter/sequence-matching.ts Outdated
Comment thread src/common/string/filter/sequence-matching.ts
Comment thread src/common/string/filter/sequence-matching.ts
Comment thread src/dialogs/quick-bar/ha-quick-bar.ts Outdated
return this._renderCommandItem(item, index);
}

if (isEntityItem(item)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just... we check if it is not a command item, we already know that, because we checked that above...?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_renderEntityItem(item) expects an EntityItem, but item is a QuickBarItem here. This would be okay except that the EntityItem interface now has the altText property.

So I'm no longer allowed to blindly send a QuickBarItem to the _renderEntityItem method due to mismatching types.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as EntityItem ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do, but I try to avoid X as Y as it weakens type safety.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but I rather do that than to add unneeded runtime checks 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okie doke

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread src/dialogs/quick-bar/ha-quick-bar.ts Outdated
@donkawechico donkawechico merged commit 538028a into dev Apr 14, 2021
@delete-merged-branch delete-merged-branch Bot deleted the refactor-sequence-matching branch April 14, 2021 22:29
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quickbar doesn't show options in other language

3 participants