RSCPlus / rscplus

RuneScape Classic client mod & preservation platform
https://rsc.plus
GNU General Public License v3.0
47 stars 35 forks source link

Dynamically-indexed settings search #162

Closed conker-rsc closed 1 year ago

conker-rsc commented 1 year ago

This was inspired by PR #150. I very much liked the UX design in that pull request, but wanted a more robust implementation.

Features:

A good place to start on this review for understanding how the code works is the javadoc for the indexSearch() method

Hubcapp commented 1 year ago

Love it, thanks for your very hard work on this. Love the implementation with getAllComponents.

I suspect we will want to go back and add search aliases to some more of the existing options e.g. "hits" "hp" and "health" are all pretty interchangeable & are used often.

Hubcapp commented 1 year ago

Maybe only two things eyebrow raising:

conker-rsc commented 1 year ago

Definitely on the aliases; I’ll add the hits stuff now but please feel free to make any other suggestions you can think of that I can get in before the initial feature merge!

Yes I started with plain regex and then figured out it wasn’t enough for some reason that I can’t remember now… I’ll take a fresh look at it again, it was one of the earlier things I did.

Noticed the header padding too (was literally one of my TODOs) but couldn’t remember if that’s something I already fixed in my FlatLAF branch and then came to the same conclusion you did, that I would deal with it there haha.

conker-rsc commented 1 year ago

Regarding the HTML; it was for un-escaping HTML values such as  ... I wanted to ensure that in the future anyone could use whatever values they would like, without us having to maintain a list, or re-invent the wheel and re-implement what libraries already do.

That said, I did a little bit more research and found that the JSoup library is capable of doing the exact same thing that I'm using it for here at about half the size (443.3kb). I tested it out and the outcome is the same as the apache library.

I'll leave it up to you whether you're okay keeping the JSoup library, or if you want me to re-implement its logic within RSC+.

EDIT: Seems like by using the JSoup library, I can get rid of the custom regex within the sanitize method, which leads to MUCH better and cleaner results from parsing as well. As such, my vote is to keep the JSoup import in place.