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.

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

allow search hits in body text that are in title of other doc #2891 #2971

Merged
merged 3 commits into from
Oct 10, 2016

Conversation

TimKam
Copy link
Member

@TimKam TimKam commented Sep 21, 2016

No description provided.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

LGTM wit nits

@@ -397,7 +397,9 @@ def stem(word):
# again, stemmer must not remove words from search index
if not _filter(stemmed_word) and _filter(word):
stemmed_word = word
if stemmed_word not in self._title_mapping and _filter(stemmed_word):
already_indexed = self._title_mapping.get(stemmed_word)\
and docname in self._title_mapping.get(stemmed_word)
Copy link
Member

Choose a reason for hiding this comment

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

I think docname in self._title.mapping.get(stemmed_word, []) is a bit simple.

@@ -420,6 +420,7 @@ var Search = {
// now check if the files don't contain excluded terms
for (file in fileMap) {
var valid = true;
//fileMap[file] = $.unique(fileMap[file]);
Copy link
Member

Choose a reason for hiding this comment

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

It seems this is not needed.

@tk0miya
Copy link
Member

tk0miya commented Sep 29, 2016

@shibukawa Do you have any opinion about this case?
In detail, please refer #2891.

@tk0miya tk0miya added this to the 1.5 milestone Sep 29, 2016
@TimKam
Copy link
Member Author

TimKam commented Sep 29, 2016

@tk0miya Thanks for the feedback! Worked it in.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

Confirmed. Thank you for updating!

@tk0miya
Copy link
Member

tk0miya commented Oct 6, 2016

If no comments until this weekend, I will merge this.

@tk0miya tk0miya merged commit de7c937 into sphinx-doc:master Oct 10, 2016
@tk0miya
Copy link
Member

tk0miya commented Oct 10, 2016

Merged.
Thank you for contribution!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants