-
Notifications
You must be signed in to change notification settings - Fork 902
Fix quadratic rendering time for many inline links #1621
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes the API for all inline patterns and processors. @facelessuser I think you have a better handle on this part of the code. Any input on that change here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I kept the search-index behavior change and documented it under Unreleased > Changed in b745bba, treating it as an inline-processor API/behavior change rather than a Fixed-only entry.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do have some mild concerns about this change. We are assuming that because the API changed here without breaking things, others using this API won't be broken. When we changed the API here in the first time, we did so in a way that was non-breaking, one you could opt into. This was done by providing a different class that could be checked. It is possible that we may need to employ some method ot indicate this is a different version of the new way. I really need to do some testing to understand the implications of the change. I will have to do some testing with this over in Pymdown Extensions, where we have a number of plugins using the new style, so I can get a better idea of what the impact here is.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that all the tests over on Pymdown Extensions passed with these changes, so that is a good sign.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @facelessuser did you intend to provide more input on this? Itis not clear to me if you had finished reviewing this.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I've just been busy. My initial pass indicated that I don't "think" it'll break anything.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can try to formally finish the review soon. |
Uh oh!
There was an error while loading. Please reload this page.