Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

clean highlightSourceLines code #66082

Merged
merged 1 commit into from
Nov 5, 2019
Merged

clean highlightSourceLines code #66082

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 54 additions & 42 deletions src/librustdoc/html/static/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,59 +163,71 @@ function getSearchElement() {

var main = document.getElementById("main");

function highlightSourceLines(ev) {
// If we're in mobile mode, we should add the sidebar in any case.
function onHashChange(ev) {
// If we're in mobile mode, we should hide the sidebar in any case.
hideSidebar();
var elem;
var search = getSearchElement();
var i, from, to, match = window.location.hash.match(/^#?(\d+)(?:-(\d+))?$/);
var match = window.location.hash.match(/^#?(\d+)(?:-(\d+))?$/);
if (match) {
from = parseInt(match[1], 10);
to = from;
if (typeof match[2] !== "undefined") {
to = parseInt(match[2], 10);
}
if (to < from) {
var tmp = to;
to = from;
from = tmp;
}
elem = document.getElementById(from);
if (!elem) {
return;
}
if (ev === null) {
var x = document.getElementById(from);
if (x) {
x.scrollIntoView();
}
}
onEachLazy(document.getElementsByClassName("line-numbers"), function(e) {
onEachLazy(e.getElementsByTagName("span"), function(i_e) {
removeClass(i_e, "line-highlighted");
});
});
for (i = from; i <= to; ++i) {
elem = document.getElementById(i);
if (!elem) {
break;
}
addClass(elem, "line-highlighted");
}
} else if (ev !== null && search && !hasClass(search, "hidden") && ev.newURL) {
return highlightSourceLines(match, ev);
Copy link
Contributor

Choose a reason for hiding this comment

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

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()

Copy link
Contributor

Choose a reason for hiding this comment

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

You could have highlightSourceLines() return whether it did work, and use that to decide if you want to show the search element.

Copy link
Member Author

Choose a reason for hiding this comment

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

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? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay that's fair, you can resolve this comment without change.

}
var search = getSearchElement();
if (ev !== null && search && !hasClass(search, "hidden") && ev.newURL) {
addClass(search, "hidden");
removeClass(main, "hidden");
var hash = ev.newURL.slice(ev.newURL.indexOf("#") + 1);
if (browserSupportsHistoryApi()) {
history.replaceState(hash, "", "?search=#" + hash);
}
elem = document.getElementById(hash);
var elem = document.getElementById(hash);
if (elem) {
elem.scrollIntoView();
}
}
}

function highlightSourceLines(match, ev) {
if (typeof match === "undefined") {
// If we're in mobile mode, we should hide the sidebar in any case.
hideSidebar();
match = window.location.hash.match(/^#?(\d+)(?:-(\d+))?$/);
}
if (!match) {
return;
}
var from = parseInt(match[1], 10);
var to = from;
if (typeof match[2] !== "undefined") {
to = parseInt(match[2], 10);
}
if (to < from) {
var tmp = to;
to = from;
from = tmp;
}
var elem = document.getElementById(from);
if (!elem) {
return;
}
if (!ev) {
var x = document.getElementById(from);
if (x) {
x.scrollIntoView();
}
}
onEachLazy(document.getElementsByClassName("line-numbers"), function(e) {
onEachLazy(e.getElementsByTagName("span"), function(i_e) {
removeClass(i_e, "line-highlighted");
});
});
for (var i = from; i <= to; ++i) {
elem = document.getElementById(i);
if (!elem) {
break;
}
addClass(elem, "line-highlighted");
}
}

function expandSection(id) {
var elem = document.getElementById(id);
if (elem && isHidden(elem)) {
Expand All @@ -234,8 +246,8 @@ function getSearchElement() {
}
}

highlightSourceLines(null);
window.onhashchange = highlightSourceLines;
highlightSourceLines();
window.onhashchange = onHashChange;

// Gets the human-readable string for the virtual-key code of the
// given KeyboardEvent, ev.
Expand Down Expand Up @@ -358,7 +370,7 @@ function getSearchElement() {
var set_fragment = function(name) {
if (browserSupportsHistoryApi()) {
history.replaceState(null, null, "#" + name);
highlightSourceLines(null);
highlightSourceLines();
} else {
location.replace("#" + name);
}
Expand Down