-
-
Notifications
You must be signed in to change notification settings - Fork 253
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
Markdown table fixes #601
Markdown table fixes #601
Conversation
Paragraph is a block level element so we would normally have a new line before it. However, here we are in a markdown cell and can't have new lines, so add a space. Otherwise words will get concatenated.
Markdown does not support colspan, but at least this way we don't lose any cell data.
Currently there was a scenario where if a cell only contains a single <p> with some text but no text directly, then vertical bars would not get appended for that cells.
@naktinis Thanks for the PR, the code looks good but tests fails in Could you please have a look, change the code (or the test if it doesn't work as intended), and add tests there for your PR if necessary? |
Added the following table tests in text format: - removing new lines in cells, - only allowing a single header row, - handling colspan by appending columns.
@adbar I believe I have now fixed the tests and added three more relating to these changes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #601 +/- ##
=======================================
Coverage 97.94% 97.94%
=======================================
Files 21 21
Lines 3498 3508 +10
=======================================
+ Hits 3426 3436 +10
Misses 72 72 ☔ View full report in Codecov by Sentry. |
@naktinis Thanks, it's much better now! I have a few questions:
|
Yes, so that each row is aware of any other rows having more columns than it does itself.
Because the previous line (that was already there before my changes) removes spaces:
|
Then I'd be in favor of removing the attribute from the output:
Could you please implement the change (or a similar one) and update the tests? |
@adbar row attribute cleanup should now be done (also updated the tests). |
Thanks! |
Contains the following fixes:
Partly fixes #599.