From 59ba74cacb87ac89f7f5fcb5233eaccb50dd8349 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 16 Dec 2022 11:12:59 -0700 Subject: rustdoc: simplify JS search routine by not messing with lev distance Since the sorting function accounts for an `index` field, there's not much reason to also be applying changes to the levenshtein distance. Instead, we can just not treat `lev` as a filter if there's already a non-sentinel value for `index`. This change gives slightly more weight to the index and path part, as search criteria, than it used to. This changes some of the test cases, but not in any obviously-"worse" way, and, in particular, substring matches are a bigger deal than levenshtein distances (we're assuming that a typo is less likely than someone just not typing the entire name). Based on https://github.com/rust-lang/rust/pull/103710#issuecomment-1296894296 --- src/librustdoc/html/static/js/search.js | 114 ++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 50 deletions(-) (limited to 'src') diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 1b8822b0b2b..88592fa0c84 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -781,7 +781,29 @@ function initSearch(rawSearchIndex) { return a - b; } - // Sort by non levenshtein results and then levenshtein results by the distance + // sort by index of keyword in item name (no literal occurrence goes later) + a = (aaa.index < 0); + b = (bbb.index < 0); + if (a !== b) { + return a - b; + } + + // Sort by distance in the path part, if specified + // (less changes required to match means higher rankings) + a = aaa.path_lev; + b = bbb.path_lev; + if (a !== b) { + return a - b; + } + + // (later literal occurrence, if any, goes later) + a = aaa.index; + b = bbb.index; + if (a !== b) { + return a - b; + } + + // Sort by distance in the name part, the last part of the path // (less changes required to match means higher rankings) a = (aaa.lev); b = (bbb.lev); @@ -810,19 +832,6 @@ function initSearch(rawSearchIndex) { return (a > b ? +1 : -1); } - // sort by index of keyword in item name (no literal occurrence goes later) - a = (aaa.index < 0); - b = (bbb.index < 0); - if (a !== b) { - return a - b; - } - // (later literal occurrence, if any, goes later) - a = aaa.index; - b = bbb.index; - if (a !== b) { - return a - b; - } - // special precedence for primitive and keyword pages if ((aaa.item.ty === TY_PRIMITIVE && bbb.item.ty !== TY_KEYWORD) || (aaa.item.ty === TY_KEYWORD && bbb.item.ty !== TY_PRIMITIVE)) { @@ -1230,15 +1239,19 @@ function initSearch(rawSearchIndex) { * * `id` is the index in both `searchWords` and `searchIndex` arrays for this element. * * `index` is an `integer`` used to sort by the position of the word in the item's name. * * `lev` is the main metric used to sort the search results. + * * `path_lev` is zero if a single-component search query is used, otherwise it's the + * distance computed for everything other than the last path component. * * @param {Results} results * @param {string} fullId * @param {integer} id * @param {integer} index * @param {integer} lev + * @param {integer} path_lev */ - function addIntoResults(results, fullId, id, index, lev) { - if (lev === 0 || (!parsedQuery.literalSearch && lev <= MAX_LEV_DISTANCE)) { + function addIntoResults(results, fullId, id, index, lev, path_lev) { + const inBounds = lev <= MAX_LEV_DISTANCE || index !== -1; + if (lev === 0 || (!parsedQuery.literalSearch && inBounds)) { if (results[fullId] !== undefined) { const result = results[fullId]; if (result.dontValidate || result.lev <= lev) { @@ -1250,6 +1263,7 @@ function initSearch(rawSearchIndex) { index: index, dontValidate: parsedQuery.literalSearch, lev: lev, + path_lev: path_lev, }; } } @@ -1280,68 +1294,68 @@ function initSearch(rawSearchIndex) { if (!row || (filterCrates !== null && row.crate !== filterCrates)) { return; } - let lev, lev_add = 0, index = -1; + let lev, index = -1, path_lev = 0; const fullId = row.id; + const searchWord = searchWords[pos]; const in_args = findArg(row, elem, parsedQuery.typeFilter); const returned = checkReturned(row, elem, parsedQuery.typeFilter); - addIntoResults(results_in_args, fullId, pos, index, in_args); - addIntoResults(results_returned, fullId, pos, index, returned); + // path_lev is 0 because no parent path information is currently stored + // in the search index + addIntoResults(results_in_args, fullId, pos, -1, in_args, 0); + addIntoResults(results_returned, fullId, pos, -1, returned, 0); if (!typePassesFilter(parsedQuery.typeFilter, row.ty)) { return; } - const searchWord = searchWords[pos]; - if (parsedQuery.literalSearch) { - if (searchWord === elem.name) { - addIntoResults(results_others, fullId, pos, -1, 0); - } - return; + const row_index = row.normalizedName.indexOf(elem.pathLast); + const word_index = searchWord.indexOf(elem.pathLast); + + // lower indexes are "better" matches + // rank based on the "best" match + if (row_index === -1) { + index = word_index; + } else if (word_index === -1) { + index = row_index; + } else if (word_index < row_index) { + index = word_index; + } else { + index = row_index; } // No need to check anything else if it's a "pure" generics search. if (elem.name.length === 0) { if (row.type !== null) { lev = checkGenerics(row.type, elem, MAX_LEV_DISTANCE + 1); - addIntoResults(results_others, fullId, pos, index, lev); + // path_lev is 0 because we know it's empty + addIntoResults(results_others, fullId, pos, index, lev, 0); } return; } if (elem.fullPath.length > 1) { - lev = checkPath(elem.pathWithoutLast, row); - if (lev > MAX_LEV_DISTANCE || (parsedQuery.literalSearch && lev !== 0)) { + path_lev = checkPath(elem.pathWithoutLast, row); + if (path_lev > MAX_LEV_DISTANCE) { return; - } else if (lev > 0) { - lev_add = lev / 10; } } - if (searchWord.indexOf(elem.pathLast) > -1 || - row.normalizedName.indexOf(elem.pathLast) > -1 - ) { - index = row.normalizedName.indexOf(elem.pathLast); - } - lev = levenshtein(searchWord, elem.pathLast); - if (lev > 0 && elem.pathLast.length > 2 && searchWord.indexOf(elem.pathLast) > -1) { - if (elem.pathLast.length < 6) { - lev = 1; - } else { - lev = 0; + if (parsedQuery.literalSearch) { + if (searchWord === elem.name) { + addIntoResults(results_others, fullId, pos, index, 0, path_lev); } - } - lev += lev_add; - if (lev > MAX_LEV_DISTANCE) { return; - } else if (index !== -1 && elem.fullPath.length < 2) { - lev -= 1; } - if (lev < 0) { - lev = 0; + + lev = levenshtein(searchWord, elem.pathLast); + + if (index === -1 && lev + path_lev > MAX_LEV_DISTANCE) { + return; } - addIntoResults(results_others, fullId, pos, index, lev); + + addIntoResults(results_others, fullId, pos, index, lev, path_lev); } /** @@ -1386,7 +1400,7 @@ function initSearch(rawSearchIndex) { return; } const lev = Math.round(totalLev / nbLev); - addIntoResults(results, row.id, pos, 0, lev); + addIntoResults(results, row.id, pos, 0, lev, 0); } function innerRunQuery() { -- cgit 1.4.1-3-g733a5 From deb05758c8d7e13599617f322be8c03b22d1d724 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 18 Jan 2023 11:41:34 -0700 Subject: rustdoc: put focus on the help link when opening it from keyboard This prevents some strange blur-event-related bugs with the "?" command by ensuring that the focus remains in the same spot when the settings area closes. --- src/librustdoc/html/static/js/main.js | 3 +++ tests/rustdoc-gui/settings.goml | 9 +++++++++ 2 files changed, 12 insertions(+) (limited to 'src') diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 9ceeeb5ae8f..6cb670d32a6 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -1082,6 +1082,9 @@ function loadCss(cssUrl) { * Show the help popup menu. */ function showHelp() { + // Prevent `blur` events from being dispatched as a result of closing + // other modals. + getHelpButton().querySelector("a").focus(); const menu = getHelpMenu(true); if (menu.style.display === "none") { window.hideAllModals(); diff --git a/tests/rustdoc-gui/settings.goml b/tests/rustdoc-gui/settings.goml index 72de41e41ba..2114e2cc4c2 100644 --- a/tests/rustdoc-gui/settings.goml +++ b/tests/rustdoc-gui/settings.goml @@ -203,6 +203,15 @@ press-key: "?" wait-for-css: ("#help-button .popover", {"display": "block"}) assert-css: ("#settings-menu .popover", {"display": "none"}) +// Now switch back to the settings popover, and make sure the keyboard +// shortcut works when a check box is selected. +click: "#settings-menu > a" +wait-for-css: ("#settings-menu .popover", {"display": "block"}) +focus: "#auto-hide-large-items" +press-key: "?" +wait-for-css: ("#settings-menu .popover", {"display": "none"}) +wait-for-css: ("#help-button .popover", {"display": "block"}) + // Now we go to the settings page to check that the CSS is loaded as expected. goto: "file://" + |DOC_PATH| + "/settings.html" wait-for: "#settings" -- cgit 1.4.1-3-g733a5 From bb5fb53b30fd2216639ae85ab44ec4445e004556 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 18 Jan 2023 11:52:31 -0700 Subject: rustdoc: fix "?" keyboard command when radio button is focused This extends the special case with checkbox settings to also cover radios. --- src/librustdoc/html/static/js/main.js | 3 ++- tests/rustdoc-gui/settings.goml | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 6cb670d32a6..604ab147f6a 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -390,7 +390,8 @@ function loadCss(cssUrl) { } if (document.activeElement.tagName === "INPUT" && - document.activeElement.type !== "checkbox") { + document.activeElement.type !== "checkbox" && + document.activeElement.type !== "radio") { switch (getVirtualKey(ev)) { case "Escape": handleEscape(ev); diff --git a/tests/rustdoc-gui/settings.goml b/tests/rustdoc-gui/settings.goml index 2114e2cc4c2..4c72ed51a49 100644 --- a/tests/rustdoc-gui/settings.goml +++ b/tests/rustdoc-gui/settings.goml @@ -212,6 +212,16 @@ press-key: "?" wait-for-css: ("#settings-menu .popover", {"display": "none"}) wait-for-css: ("#help-button .popover", {"display": "block"}) +// Now switch back to the settings popover, and make sure the keyboard +// shortcut works when a check box is selected. +click: "#settings-menu > a" +wait-for-css: ("#settings-menu .popover", {"display": "block"}) +wait-for-css: ("#help-button .popover", {"display": "none"}) +focus: "#theme-system-preference" +press-key: "?" +wait-for-css: ("#settings-menu .popover", {"display": "none"}) +wait-for-css: ("#help-button .popover", {"display": "block"}) + // Now we go to the settings page to check that the CSS is loaded as expected. goto: "file://" + |DOC_PATH| + "/settings.html" wait-for: "#settings" -- cgit 1.4.1-3-g733a5 From 9ee4df0e9cbe474a0f357ed00dd479a2dae65e23 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 18 Jan 2023 12:39:13 -0700 Subject: rustdoc: remove redundant rule `#settings .setting-line` Since the current version of settings.js always nests things below a div with ID `settings`, this rule always overrode the one above. --- src/librustdoc/html/static/css/settings.css | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'src') diff --git a/src/librustdoc/html/static/css/settings.css b/src/librustdoc/html/static/css/settings.css index 3fa47875173..7211ffb7795 100644 --- a/src/librustdoc/html/static/css/settings.css +++ b/src/librustdoc/html/static/css/settings.css @@ -1,5 +1,5 @@ .setting-line { - margin: 0.6em 0 0.6em 0.3em; + margin: 1.2em 0.6em; position: relative; } @@ -55,10 +55,6 @@ cursor: pointer; } -#settings .setting-line { - margin: 1.2em 0.6em; -} - .setting-line .radio-line input:checked { box-shadow: inset 0 0 0 3px var(--main-background-color); background-color: var(--settings-input-color); -- cgit 1.4.1-3-g733a5