Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Allow searching for multiple terms#173

Closed
wormingdead wants to merge 9 commits into
PathOfBuildingCommunity:dev from
wormingdead:multiple-search-li
Closed

Allow searching for multiple terms #173
wormingdead wants to merge 9 commits into
PathOfBuildingCommunity:dev from
wormingdead:multiple-search-li

Conversation

@wormingdead

@wormingdead wormingdead commented Feb 13, 2020

Copy link
Copy Markdown

This pull request mirrors Openarl/PathOfBuilding#1853. There's been no feedback there, so I thought I'd try here.


This series of commits allow the search bar to handle multiple search terms. When the user searches with the string, bleed|frenzy, nodes that contain text with either bleed or frenzy will be highlighted. In other words, this change allows searching the union of each term. This is a change from the previous functionality of PoB searching only for the entire search string, including any whitespace. For comparison, the game allows whitespace to separate search terms and performs a search of the intersection of each term.

There are two big changes that should be considered before accepting this pull request.


1. The search cache has been changed from a table of booleans to a table of bitmaps (integers)

I don't know how this will work with the current serialization method. From reading the code, it looks like only searchStr, not searchStrResults, is serialized and saved, but I may be misunderstanding something. My testing did not show any issues.


2. Users can no longer search for | (the pipe symbol)

I don't think this will affect anyone.


Some other comments:

  • ebc5996 This commit may need to be reverted. I wasn't sure why the search term was case-sensitive for this comparison.
  • The search is currently limited to just eight terms. Additional terms will be ignored.

Thank you for any feedback.

Split searchStr on searchTermSentinal. Change searchStrResults to a
bitmap table. Draw a circle for each successful search term. Color
highlight circles with searchResultColormap.
Require luabit for access to bitwise functions
@wormingdead wormingdead changed the base branch from master to dev February 28, 2020 02:09
@LocalIdentity LocalIdentity added the enhancement New feature, calculation, or mod label Feb 29, 2020

Copy link
Copy Markdown
Contributor

After talking with the team, we have decided this is not a feature we are interested in supporting currently.

elindred reacted with thumbs down emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement New feature, calculation, or mod

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /