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

Rework autorefs replacement to not re-parse the final HTML #188

Merged
merged 1 commit into from
Dec 6, 2020

Conversation

oprypin
Copy link
Member

@oprypin oprypin commented Dec 6, 2020

Define a "transport representation" for autorefs:

<span data-mkdocstrings-identifier="SomeIdentifier">SomeHTML</span>

First a native Markdown extension translates from the usual [SomeMarkdown][SomeIdentifier] to the above, and then the post-process replacement mechanism (which is kept in the same place as before) doesn't need to be so careful and complicated, it just indiscriminately replaces such exact strings.

This is a very big boost in performance and I think is more future-proof.

Other mkdocstrings handlers are also free to use this mechanism: define whatever syntax for autorefs that they need and then insert this exact HTML themselves, for the postprocessing to pick up later. It used to be possible to insert the square-brackets Markdown before, but that was very fragile.

Fixes #187.

Define a "transport representation" for autorefs:

    <span data-mkdocstrings-identifier="SomeIdentifier">SomeHTML</span>

First a native Markdown extension translates from the usual `[SomeMarkdown][SomeIdentifier]` to the above, and then the post-process replacement mechanism (which is kept in the same place as before) doesn't need to be so careful and complicated, it just indiscriminately replaces such exact strings.

This is a very big boost in performance and I think is more future-proof.

Other mkdocstrings handlers are also free to use this mechanism: define whatever syntax for autorefs that they need and then insert this exact HTML themselves, for the postprocessing to pick up later. It used to be possible to insert the square-brackets Markdown before, but that was very fragile.
@oprypin
Copy link
Member Author

oprypin commented Dec 6, 2020

Fixes #187.

Compare the performance profile to what I posted there.

image

oprypin added a commit to mkdocstrings/crystal that referenced this pull request Dec 6, 2020
Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

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

Well @oprypin, this is absolutely fantastic! Thank you so much!

Less code, more robust, future-proof, new tests, and you got rid of beautifulsoup... this is great 🚀 !

@pawamoy pawamoy merged commit 22a9e4b into mkdocstrings:master Dec 6, 2020
oprypin added a commit to mkdocstrings/crystal that referenced this pull request Dec 7, 2020
oprypin added a commit to oprypin/mkdocstrings that referenced this pull request Dec 19, 2020
First introduced in mkdocstrings#188, there's a regression: if something messes with the final HTML before mkdocstrings plugin gets to it (as happens specifically with 'minify' plugin if it ever appears before our plugin in the config -- which it shouldn't anyway).

So workaround this specific case. Make the attribute quotes optional to also catch minified HTML.
Hard to do much else, though perhaps a warning would make sense.
oprypin added a commit to oprypin/mkdocstrings that referenced this pull request Dec 19, 2020
First introduced in mkdocstrings#188, there's a regression: if something messes with the final HTML before mkdocstrings plugin gets to it, it can't be detected and replaced.
A known case is with 'minify' plugin if it appears before our plugin in the config (which it shouldn't anyway).

So workaround this specific case. Make the attribute quotes optional to also catch minified HTML.
Hard to do much else, though perhaps a warning would make sense.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix_refs is by far the slowest part of this
2 participants