Skip to content

Conversation

pieterprovoost
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.93% when pulling 51d700c on pieterprovoost:emptytable into cf7234d on waylan:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.93% when pulling 51d700c on pieterprovoost:emptytable into cf7234d on waylan:master.

@waylan
Copy link
Member

waylan commented Apr 4, 2015

@pieterprovoost thank you for your submission. However, before I accept your PR, please review my response to #74. Does the change you made more closely match the behavior of tables in PHP Markdown Extra? If so, could you provide an example table (preferably as a test that fails without your change and passes with it). If not, then may I suggest you fork the existing extension and make it do whatever you want -- or perhaps a third party extension already exists that meets your needs.

@pieterprovoost
Copy link
Contributor Author

@waylan PHP Markdown supports zero rows, so this change does make Python Markdown more like PHP Markdown. This test fails with the current version:

import markdown
html = markdown.markdown("|a|b|\n|--|--|\n", extensions=['tables'])
assert("<table>" in html) 

@waylan
Copy link
Member

waylan commented Apr 5, 2015

Thanks. If you add a test here, I'll accept it.

@pieterprovoost
Copy link
Contributor Author

@waylan I added the test to my branch, but it's not picked up? https://github.com/pieterprovoost/Python-Markdown/commit/47737e4a515e63e21bbfd2d2b35404fe44432890

@waylan
Copy link
Member

waylan commented Apr 6, 2015

Well, you didn't also update the corresponding HTML file which provides the expected output for the test to compare against. If you run run-tests.py update the updated test should be found and the HTML file automaticaly regenerated.

Although now that I take a second look, I see that the tests are passing, which is odd. Does the update command detect the change? Or do I have a bug in the testing framework?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.93% when pulling 8cd0446 on pieterprovoost:emptytable into cf7234d on waylan:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.93% when pulling 8cd0446 on pieterprovoost:emptytable into cf7234d on waylan:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.93% when pulling 8cd0446 on pieterprovoost:emptytable into cf7234d on waylan:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.93% when pulling 8cd0446 on pieterprovoost:emptytable into cf7234d on waylan:master.

waylan added a commit that referenced this pull request Apr 6, 2015
@waylan waylan merged commit 3481625 into Python-Markdown:master Apr 6, 2015
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