Skip to content

Make search exclude files a path operation #5247

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

Closed
agjohnson opened this issue Feb 7, 2019 · 2 comments · Fixed by #7308
Closed

Make search exclude files a path operation #5247

agjohnson opened this issue Feb 7, 2019 · 2 comments · Fixed by #7308
Labels
Accepted Accepted issue on our roadmap Bug A bug
Milestone

Comments

@agjohnson
Copy link
Contributor

Our method of filtering out files from search operations was a string comparison, which produced a lot of false positive hits. This comparison should be a path comparison that looks explicitly for os.path.join(doc_path, 'search.html') instead of using string comparison.

Filtering is disabled for now, we should clean up search results soon and reenable it.

Refs #5246

@agjohnson agjohnson added Bug A bug Accepted Accepted issue on our roadmap labels Feb 7, 2019
@agjohnson agjohnson added this to the 3.2 milestone Feb 7, 2019
@stsewd stsewd modified the milestones: 3.2, 3.4 Mar 6, 2019
@stsewd stsewd modified the milestones: 3.4, 3.5 Mar 27, 2019
@dojutsu-user
Copy link
Member

I would like to work on this.
Can I get more info on this... it is not clear to me.

@agjohnson
Copy link
Contributor Author

Here is the main issue:
https://github.com/rtfd/readthedocs.org/pull/5246/files#diff-7faf5e6b86776ca9b80505230b669285R114

It simply looked for paths ending search.html for example. This would incorrectly catch files like foo/search.html or foo_search.html.

If we can't use file operations for this query in some way, we should at least be more strict about the root path. We should more explicitly catch the default sphinx pages we're not looking to index.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug A bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants