Skip to content

Conversation

karepker
Copy link
Contributor

@karepker karepker commented Jun 1, 2015

Changed hard-coded numbers assuming tab length to be 4 to use self.tab_length.

This change applies to he default parsers in mardown/blockprocessors.py and those in markdown/extensions/sane_lists.py.

All current tests should pass.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't super be called here (and in all other subclassesd below)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you targeting compatibility with Python 2? If so, I'm not sure super() would work in this context, because using it requires new style classes.

Also, a call to super() seems like it would be inconsistent in the code base. In ListIndentProcessor.__init__(...) method (line 144 in markdown/blockprocessors.py), there is a call to BlockProcessor.__init__(...).

Copy link
Member

Choose a reason for hiding this comment

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

Looks like I missed updating a class to new style classes in 794bffb. As the release notes stated (for 2.3 -- that was a while ago):

All classes are now “new-style” classes. In other words, all classes subclass from ‘object’.

However, I can clearly see that Blockprocessor does not. Looks like this whole file needs to be updated to be consistent with the rest of the library. If want to do that, great. If not, I'll take care of it myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'm happy to make this change. I'll update the pull request shortly.

@waylan
Copy link
Member

waylan commented Jun 1, 2015

@karepker if you address my two comments and fix the white space issues raised by the flake8 test I'll be happy to merge this.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 92.95% when pulling cced40a on karepker:master into e594213 on waylan:master.

@coveralls
Copy link

coveralls commented Jun 1, 2015

Coverage Status

Coverage increased (+0.02%) to 92.953% when pulling cced40a on karepker:master into e594213 on waylan:master.

@karepker
Copy link
Contributor Author

karepker commented Jun 1, 2015

See my questions regarding the use of super(). I'll make the other changes.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 92.95% when pulling d3cc989 on karepker:master into e594213 on waylan:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 92.95% when pulling d3cc989 on karepker:master into e594213 on waylan:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 92.95% when pulling 99257e4 on karepker:master into e594213 on waylan:master.

waylan added a commit that referenced this pull request Jun 2, 2015
Fixed #414 parser ignoring value of `tab_length`
@waylan waylan merged commit c7410d1 into Python-Markdown:master Jun 2, 2015
@waylan
Copy link
Member

waylan commented Jun 2, 2015

Awesome. Thanks.

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.

3 participants