-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add filename filter for performance gain #26
base: main
Are you sure you want to change the base?
Conversation
utils.normalize_url("assets/javascripts/"), page.url | ||
# No tags found, we can return earlier | ||
if len(swagger_ui_list) == 0: | ||
return output |
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.
The whole block down this change is just un-indented.
The diff is a bit weird on that, sorry.
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.
LGTM
7145cf5
to
ade0f07
Compare
I've added a test and reviewed the failing ones. |
utils.normalize_url("assets/javascripts/"), page.url | ||
# No tags found, we can return earlier | ||
if len(swagger_ui_list) == 0: | ||
return output |
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.
LGTM
@@ -56,6 +65,10 @@ class SwaggerUIPlugin(BasePlugin): | |||
("validatorUrl", config_options.Type(str, default="none")), | |||
("extra_css", config_options.Type(list, default=[])), | |||
("dark_scheme_name", config_options.Type(str, default="slate")), | |||
( | |||
"filter_files", |
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.
I renamed the option to filter_files. I think using the file path is less ambiguous.
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.
What about marking files on the markdown frontmatter? (see here) The frontmatter data is already available in the page event hooks. I didn't though about that before, but it feels more ergonomic in general.
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.
I'm good with either way
This PR:
closes: #25