Skip to content
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

Allow table marker row to have more columns than header #253

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christianselig
Copy link

To match with Reddit's more lax table header/marker syntax (the header being the first row, and the marker row being the row after it that dictates the alignments), this change allows more marker/alignment columns than header columns.

For whatever reason this behavior/syntax is very common on Reddit, I assume because some folks will just edit the header row to remove a column, and then simply not update the alignment/marker row.

For instance this syntax renders a table on Reddit with 3 columns, but does not render with cmark-gfm as it has one too many marker/alignment columns:

markdown

|Header1|Header2|Header3|
|-------|-------|-------|-------|
| Body1 | Body2 | Body 3|

But does render on Reddit as if the last marker/alignment column wasn't present, thus looking like this:

Header1 Header2 Header3
Body1 Body2 Body 3

This PR changes the rules of the table extension to allow more marker/alignment columns than header columns.

It's a seemingly harmless change and all tests still pass. Note that the opposite (more header columns than marker/alignment columns) would fail the tests understandably as some header columns would not have an explicit alignment set.

I totally understand if this isn't behavior you're interested in/care about, I just integrated it into my app and appreciate this library and thus wanted to do the good thing and offer up my work in case you were interested!

(Shoutout to @QuietMisdreavus for helping me, any nice parts of this PR are thanks to her, and any mistakes are totally my fault.)

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.

1 participant