-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Draft: Add test filesection indexer #11791
base: main
Are you sure you want to change the base?
Conversation
This is a test implementation of storing the section data in a similar way to filetreediff. The goal would be to build a command pallette that initially lets users do some basic filtering of just the filenames.
if not manifest: | ||
return None | ||
|
||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example in dev from Addons.
{
"enabled": true,
"sections": [
{
"path": "genindex.html",
"sections": [
{
"id": "index",
"title": "Index"
}
]
},
{
"path": "index.html",
"sections": [
{
"id": "read-the-docs-addons",
"title": "🦸 Read the Docs Addons"
},
{
"id": "examples",
"title": "Examples"
},
{
"id": "contents",
"title": "Contents"
}
]
},
{
"path": "releasing.html",
"sections": [
{
"id": "releasing",
"title": "Releasing"
}
]
},
{
"path": "search.html",
"sections": [
{
"id": "",
"title": "Search - Read the Docs Addons documentation"
}
]
},
{
"path": "testing.html",
"sections": [
{
"id": "testing",
"title": "Testing"
},
{
"id": "localhost",
"title": "localhost"
},
{
"id": "read-the-docs",
"title": "Read the Docs"
}
]
}
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't return the content of manifest here, but instead, we should return the URL where the frontend would download it and parse it on the client side. Otherwise, this response will be huge and most of the times it's not going to be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, I think we will probably want to do the same for filetreediff as well; since there will be PRs will a ton of files changed.
Besides, that will help us to invalidate the cache for filetreediff when a new build is done, but keep cached the /_/addons/
endpoint for the whole project, as we are currently doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I just copied the implementation from filetreediff. I figured in production we would only pull the data when somebody pulls up the command pallet, but this made it easy to test.
In theory, the data is also already in our search index, I just wasn't 100% sure how to get it out of elastic.
path=page["path"], | ||
sections=[FileSection(**section) for section in page["sections"]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably makes sense to save the title of the page itself as well -- so we don't show release.html
only but, Releasing Addons :: release.html
or similar.
This is a test implementation of storing the section data in a similar way to filetreediff.
The goal would be to build a command pallette that initially lets users do some basic filtering of just the filenames.
Demo
Screen.Recording.2024-11-22.at.4.04.47.PM.mov