Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
5 changes: 4 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ Features added
Bugs fixed
----------

* #11959: Fix multiple term matching when word appears in both title and document.
* #11961: HTML Search: Fix duplicated search results.
Patch by Will Lachance.
* #11959: HTML Search: Fix multiple term matching when word appears in both
title and document.
Patch by Will Lachance.
* #11958: HTML Search: Fix partial matches overwriting full matches.
Patch by William Lachance.
Expand Down
43 changes: 30 additions & 13 deletions sphinx/themes/basic/static/searchtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
if (typeof Scorer === "undefined") {
var Scorer = {
// Implement the following function to further tweak the score for each result
// The function takes a result array [docname, title, anchor, descr, score, filename]
// The function takes a result array [docname, title, anchor, anchorIsDocumentTitle, descr, score, filename]
// and returns the new score.
/*
score: result => {
const [docname, title, anchor, descr, score, filename] = result
const [docname, title, anchor, anchorIsDocumentTitle, descr, score, filename] = result
return score
},
*/
Expand Down Expand Up @@ -64,7 +64,7 @@ const _displayItem = (item, searchTerms, highlightTerms) => {
const showSearchSummary = DOCUMENTATION_OPTIONS.SHOW_SEARCH_SUMMARY;
const contentRoot = document.documentElement.dataset.content_root;

const [docName, title, anchor, descr, score, _filename] = item;
const [docName, title, anchor, _anchorIsDocumentTitle, descr, score] = item;

let listItem = document.createElement("li");
let requestUrl;
Expand Down Expand Up @@ -198,7 +198,8 @@ const Search = {
if (Search._queued_query !== null) {
const query = Search._queued_query;
Search._queued_query = null;
Search.query(query);
let { results, searchTerms, highlightTerms } = Search.query(query);
_displayNextItem(results, results.length, searchTerms, highlightTerms);
}
},

Expand Down Expand Up @@ -246,8 +247,12 @@ const Search = {
Search.startPulse();

// index already loaded, the browser was quick!
if (Search.hasIndex()) Search.query(query);
else Search.deferQuery(query);
if (Search.hasIndex()) {
let { results, searchTerms, highlightTerms } = Search.query(query);
_displayNextItem(results, results.length, searchTerms, highlightTerms);
} else {
Search.deferQuery(query);
}
},

/**
Expand Down Expand Up @@ -304,10 +309,13 @@ const Search = {
if (title.toLowerCase().trim().includes(queryLower) && (queryLower.length >= title.length/2)) {
for (const [file, id] of foundTitles) {
let score = Math.round(100 * queryLower.length / title.length)
let anchorIsDocumentTitle = titles[file] === title
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we pre-calculate this when building the alltitles in Python, so that the client doesn't have to transform each entry every time?

Copy link
Contributor

Choose a reason for hiding this comment

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

This appears theoretically possible. I've drafted some sample code to illustrate that alternative here: #12047 - however it does adjust the contents of the index, and that could be considered a disadvantage.

let anchor = id ? `#${id}` : ""
results.push([
docNames[file],
titles[file] !== title ? `${titles[file]} > ${title}` : title,
id !== null ? "#" + id : "",
anchorIsDocumentTitle ? title : `${titles[file]} > ${title}`,
anchor,
anchorIsDocumentTitle,
null,
score,
filenames[file],
Expand All @@ -324,6 +332,7 @@ const Search = {
results.push([
docNames[file],
titles[file],
false,
id ? "#" + id : "",
null,
score,
Expand All @@ -348,8 +357,8 @@ const Search = {
// display function below uses pop() to retrieve items) and then
// alphabetically
results.sort((a, b) => {
const leftScore = a[4];
const rightScore = b[4];
const leftScore = a[5];
const rightScore = b[5];
if (leftScore === rightScore) {
// same score: sort alphabetically
const leftTitle = a[1].toLowerCase();
Expand All @@ -364,7 +373,13 @@ const Search = {
// note the reversing of results, so that in the case of duplicates, the highest-scoring entry is kept
let seen = new Set();
results = results.reverse().reduce((acc, result) => {
let resultStr = result.slice(0, 4).concat([result[5]]).map(v => String(v)).join(',');
// de-duplicate on file, title, description, and (if not the title section) anchor
// we omit the anchor for the title section as otherwise we'll get two entries for the
// entire document
let [docname, title, anchor, anchorIsDocumentTitle, descr, score, filename] = result;
// Consider a link to the anchor representing the document title equivalent to a link
// to the document without an anchor
let resultStr = [docname, title, anchorIsDocumentTitle ? '' : anchor, descr].map(v => String(v)).join(',');
if (!seen.has(resultStr)) {
acc.push(result);
seen.add(resultStr);
Expand All @@ -378,8 +393,7 @@ const Search = {
//Search.lastresults = results.slice(); // a copy
// console.info("search results:", Search.lastresults);

// print the results
_displayNextItem(results, results.length, searchTerms, highlightTerms);
return { results, searchTerms, highlightTerms };
},

/**
Expand Down Expand Up @@ -440,6 +454,7 @@ const Search = {
docNames[match[0]],
fullname,
"#" + anchor,
false,
descr,
score,
filenames[match[0]],
Expand Down Expand Up @@ -474,6 +489,7 @@ const Search = {
{ files: terms[word], score: Scorer.term },
{ files: titleTerms[word], score: Scorer.title },
];

// add support for partial matches
if (word.length > 2) {
const escapedWord = _escapeRegExp(word);
Expand Down Expand Up @@ -550,6 +566,7 @@ const Search = {
docNames[file],
titles[file],
"",
false,
null,
score,
filenames[file],
Expand Down
8 changes: 8 additions & 0 deletions tests/js/documentation_options.js
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
const DOCUMENTATION_OPTIONS = {};

// stub Stemmer / stopWords
function Stemmer() {
this.stemWord = function (word) {
return word;
}
};
let stopwords = [];
27 changes: 27 additions & 0 deletions tests/js/searchtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('Basic html theme search', function() {
"index",
"<no title>",
"",
false,
null,
5,
"index.rst"
Expand Down Expand Up @@ -48,6 +49,7 @@ describe('Basic html theme search', function() {
'index',
'Main Page',
'',
false,
null,
15,
'index.rst']];
Expand All @@ -56,6 +58,31 @@ describe('Basic html theme search', function() {

});

describe('query', function() {
it("should not duplicate on title and content", function() {
index = {
alltitles: {
'Main Page': [[0, 'main-page']],
},
docnames:["index"],
filenames:["index.rst"],
indexentries:{},
objects:{},
objtypes: {},
objnames: {},
terms:{main:0, page:0},
titles:["Main Page"],
titleterms:{ main:0, page:0 }
}
Search.setIndex(index);
let { results } = Search.query('main page');
// should only be one result
expect(results).toEqual([
[ 'index', 'Main Page', '#main-page', true, null, 100, 'index.rst' ],
]);
});
})

});

describe("htmlToText", function() {
Expand Down