Website issue tracker link and better search performance #6483
Conversation
* last minor improvements
|
r? @Manishearth (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @killercup since you wrote the lint listing page |
There was a problem hiding this comment.
Thanks for the ping Manish!
This looks solid -- the only question I have before approving: Is the " "->"_" thing I commented on on purpose?
Also I can't belive there are enough lints an docs now to make the naive solution slow. Great work everyone! This is an amazing problem to have!
| @@ -216,40 +233,31 @@ <h4 class="list-group-item-heading"> | |||
| }; | |||
|
|
|||
| $scope.bySearch = function (lint, index, array) { | |||
| let search_str = $scope.search; | |||
| let searchStr = $scope.search; | |||
|
|
||
| // Search by id | ||
| let id_search = search_str.trim().replace(/(\-| )/g, "_"); | ||
| if (lint.id.includes(id_search)) { | ||
| if (lint.id.indexOf(searchStr.replace("-", "_")) !== -1) { |
There was a problem hiding this comment.
The previous code also replaced spaces with underscores. Not sure if this is still desired but I at least wanted to point it out :)
There was a problem hiding this comment.
Jup this is on purpose. The space search is now covered by line 252 if (lint.id.indexOf(therms[index]) !== -1) this allows the words to be out of order which is likely when somebody just types them in. It could also reduce performance since we are not using a regex in the replace function, but I'm not sure about that :)
There was a problem hiding this comment.
Ah of course, that makes sense!
(I would expect any decent JS enngine to optimize the simple regex to a byte match so it's probably as fast as the literal replace.)
util/gh-pages/index.html
Outdated
| @@ -180,6 +181,22 @@ <h4 class="list-group-item-heading"> | |||
| } | |||
| } | |||
|
|
|||
| function searchLint(lint, therm) { | |||
There was a problem hiding this comment.
Thanks for pulling this out. As an aside, I think therm should be term here and below?
There was a problem hiding this comment.
Spelling is not always my strong suite 😅 thanks for pointing that out. It should be fixed now 🙃
|
|
||
| // Search by id | ||
| let id_search = search_str.trim().replace(/(\-| )/g, "_"); | ||
| if (lint.id.includes(id_search)) { | ||
| if (lint.id.indexOf(searchStr.replace("-", "_")) !== -1) { |
There was a problem hiding this comment.
Ah of course, that makes sense!
(I would expect any decent JS enngine to optimize the simple regex to a byte match so it's probably as fast as the literal replace.)
|
@bors r+ |
|
📌 Commit f055e7f has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
This PR implements some improvements to the website:
Added a "Search on Github" link to the "Known problems" section (Closes idea: put link to github ticket search into "known problems" field #5386)
Another mock up I created with the GitHub logo
Only starting the search after three letters and improving the search performance in general. (Followup Adapted the website search for better matching #6477)
Testing
These changes can be tested locally by:
util/gh-pages/index.htmland open the html file. Make sure that it can load the lint data. (Browsers can be a bit iffy when opening a local html page and loading data)Sources for search performance:
indexOfseams to be the best)Note: The performance is still a bit poor when going from a specific lint to no search filter. I suspect that angular is recreating all lint items when the filter is cleared causing a major lag spike. The filter functions is at least optimized for little to no search.
changelog: Added a "Search on GitHub" link to the website