-
Notifications
You must be signed in to change notification settings - Fork 871
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
Feature ancestry #598
Feature ancestry #598
Conversation
Intial work and tests for excluding a pattern due parental tags.
1. If initalizing the inline processor with an element, allow passing in a list of that elements parents. 2. Update the parent map along the way so that children added to the stack will have their ancestry built proper from the parent_map. 3. Move some poorly placed pushing and popping of the ancestry stack to more appropriate places.
Document the new getExcludes method.
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.
Overall it looks good. I'm leaving for a week tomorrow so I probably won't look at this again until next weekend at the earliest.
docs/extensions/api.txt
Outdated
|
||
Returns an array of tag names that are undesirable ancestors. The pattern | ||
should not match if it would cause the content to be a descendant of one | ||
of the tag names in the list. |
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.
"Returns an array... tag names in the list." Let's be consistent here. And in Python we call them lists, not arrays.
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 work in many languages, so things blur together at times and I make mistakes like this.
markdown/inlinepatterns.py
Outdated
@@ -207,6 +207,10 @@ def __init__(self, pattern, markdown_instance=None): | |||
if markdown_instance: | |||
self.markdown = markdown_instance | |||
|
|||
def getExcludes(self): |
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.
Any reason why this is a method? The end user of API (extension developer) is defining a list, not "getting" the list. The fact that the list is being retrieved is not relevant to the user. Rather, the user is interested in defining what to exclude. Why not a property exclude = [...]
? Does it even need to be specific to an instance of the class?
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.
Something like exclude
is fine (maybe something a bit more unique in case existing extensions happen to use that name for something else). We can make the default a tuple to be safe so no one will accidentally modify the base list and ripple through all instances that use the base. That will force people to override the default opposed to appending to it.
To be safe, we can probably just wrap the treeprocessor access with a try/except to be safe as well.
Remove `getExcludes` and replace with a class attribute `ANCESTOR_EXCLUDES`.
Suggested changes have been made, so I'm all done unless there are more suggestions. Whenever you get back, just let me know if there is anything else that needs to be addressed. |
The magiclink extension uses ancestry exclusion to prevent auto-linking when text is already inside a link. Support for this was only added to Markdown in version 2.6.10 (see Python-Markdown/markdown#598).
Feature adds the ability for an inline pattern to define a list of ancestor tag names that should be avoided. If a pattern would create a descendant of one of the listed tag names, the pattern will not match.