clean highlightSourceLines code #66082
Conversation
| addClass(elem, "line-highlighted"); | ||
| } | ||
| } else if (ev !== null && search && !hasClass(search, "hidden") && ev.newURL) { | ||
| return highlightSourceLines(match, ev); |
There was a problem hiding this comment.
If you simply called this unconditionally, passing undefined for match then you wouldn't need to duplicate the match expression in this function and in hilightSourceLines -- also you'd be able to eliminate the duplication of the call to hideSidebar()
There was a problem hiding this comment.
You could have highlightSourceLines() return whether it did work, and use that to decide if you want to show the search element.
There was a problem hiding this comment.
I'm not a big fan of the idea... :-/ If it matches, then we shouldn't look anywhere else. The code is more clear from my point of view in this way. Also, what would the boolean mean? If we encounter an error while trying to highlight the lines, should we return true or false? :)
There was a problem hiding this comment.
Okay that's fair, you can resolve this comment without change.
There was a problem hiding this comment.
With @GuillaumeGomez 's raised concern, I withdraw my refactor suggestion.
I have not been able to confirm this still works though, so that's on the author :D
|
I can confirm it works locally so let's go! @bors: r=kinnison rollup |
|
📌 Commit 1c78af7 has been approved by |
bors
added
the
S-waiting-on-bors
…rcelines, r=kinnison clean highlightSourceLines code This is the first part of rust-lang#66046. Now that I've splitted the hashchange stuff and the source code lines highlighting, I'll be able to fix the whole issue once and for all. r? @kinnison
Rollup of 8 pull requests Successful merges: - #65948 (Improve MaybeUninit::get_{ref,mut} documentation) - #65953 (Allow specifying LLVM's MCTargetOptions::ABIName in target specification files) - #66012 (De-querify `trivial_dropck_outlives`.) - #66025 (`Span` cannot represent `span.hi < span.lo`) - #66047 (Don't double-count `simd_shuffle` promotion candidates) - #66053 (when Miri tests are not passing, do not add Miri component) - #66082 (clean highlightSourceLines code) - #66091 (Implemented the home_dir for VxWorks) Failed merges: r? @ghost
…nts, r=Dylan-DPC No more hidden elements Fixes rust-lang#66046. Follow-up of rust-lang#66082. r? @kinnison
This is the first part of #66046. Now that I've splitted the hashchange stuff and the source code lines highlighting, I'll be able to fix the whole issue once and for all.
r? @kinnison