Skip to content

Fix complex scenarios with definition, ordered, and unordered lists #1007

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

Merged
merged 3 commits into from
Jul 27, 2020

Conversation

facelessuser
Copy link
Collaborator

Adds logic to the definition list indentation parser to be aware of unordered and ordered lists.

Fixes #918

@facelessuser
Copy link
Collaborator Author

I want to look into why coverage isn't like I expect. It seems the very minimum to get the tests to pass is to add ul and ol to the def list indention processor and li as well. With that said, none of the tests in the entire code base seem to exercise the create_item function. I want to try and force that area if possible to see what it actually does. Right now, I think it may not do what I think it does. And to be honest, it may never get executed at all, but I need to at least explore and understand what is supposed to trigger it.

The entire test suite never exercises 'create_item', not the original
in blockprocessors or the override in def_list. Uncertain of how to even
trigger the code to test logic. Remove changes as they are untestable
currently.
@facelessuser
Copy link
Collaborator Author

@waylan I removed changes to def_list's create_item function. Nothing in the test suite exercises that code, not in the original, which def_list derives from, nor in the def_list one. I'm not even sure it can be triggered. Maybe it did at one time 🤷 .

Regardless, it makes no sense for me to alter code that can not be validated, isn't validated, and probably never actually run. I figure if a scenario comes up in the future that can actually trigger the code, and there is a problem, we can fix it then.

At this point, I think I'm done with this pull.

@waylan
Copy link
Member

waylan commented Jul 27, 2020

I did a quick search, and it appears the only place create_item is called is here:

else:
self.create_item(sibling, block)
self.parser.state.reset()

As it turns out, line 204 is listed among the lines with no test coverage, even for ol and ul lists. It appears that it is simply a safety net which was added at the end of a bunch of elif statements. My guess is it would never get called. Which begs the question: why was it ever added to the def_list extension?

In any event, thanks for the work on this.

@waylan waylan merged commit 370e17b into Python-Markdown:master Jul 27, 2020
@facelessuser
Copy link
Collaborator Author

It's possible that at some point it did get exercised. Whatever the case, whether just a safety net, or made that way over time, it doesn't seem necessary today 🙂.

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.

definition lists containing multi-paragraph ordered lists rendered as code
2 participants