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

Add table parser #11

Merged
merged 15 commits into from
Jul 30, 2022
Merged

Add table parser #11

merged 15 commits into from
Jul 30, 2022

Conversation

karlb
Copy link
Owner

@karlb karlb commented Jul 23, 2022

By adding the code from #5.

Remaining TODO:

  • Find out why the table parser does not proceed past the headings in the test case
  • Add table to documentation

Closes #5.

@karlb karlb marked this pull request as draft July 23, 2022 20:07
it breaks paragraph handling
pointer arithmetic are only allowed to go 1 past the end of the array.
going any further than that is UB.

	char buf[8];
	char *p = buf + sizeof buf; /* OK to compute, not ok to dereference */
	char *q = buf + sizeof buf + 1; /* INVALID! neither OK compute nor dereference */

> If both the pointer operand and the result point to elements of the
> same array object, or one past the last element of the array object,
> the evaluation shall not produce an overflow; otherwise, the behavior
> is undefined.

ref: https://port70.net/~nsz/c/c11/n1570.html#6.5.6p8
@N-R-K
Copy link
Collaborator

N-R-K commented Jul 27, 2022

Find out why the table parser does not proceed past the headings in the test case

Should be fixed by 492b141

Scratch that, just noticed that the cells aren't properly aligned on the first table.

@N-R-K
Copy link
Collaborator

N-R-K commented Jul 27, 2022

First time seeing git diff being used for catching regression in tests. Very lateral thinking, I like it.

inrow is assigned `-1` to indicate weather it's inside heading or not.
however "plain char" is not guaranteed to be signed. explicitly use
`signed char` instead.

> whether a ''plain'' char is treated as signed is
> implementation-defined.

ref: https://port70.net/~nsz/c/c11/n1570.html#6.3.1.1p3
Copy link
Collaborator

@N-R-K N-R-K left a comment

Choose a reason for hiding this comment

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

Scratch that, just noticed that the cells aren't properly aligned on the first table.

Should be fixed for real now.


A couple other notes...

tests/table.html Outdated Show resolved Hide resolved
<p>And here is another table</p>
<table>
<tr><th>Heading 1 </th><th>Some other heading </th></tr>
<tr><td>I am a table cell. </td><td>Me too! </td></tr>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Both in the heading and the entries, we retain any trailing white-spaces. Most other converters strip any trailing white-spaces.

tests/table.html Outdated Show resolved Hide resolved
N-R-K and others added 3 commits July 29, 2022 14:09
most other parsers either use `align` or use `style="text-align: "`
@karlb karlb marked this pull request as ready for review July 30, 2022 07:59
@karlb karlb merged commit e51231b into master Jul 30, 2022
@karlb
Copy link
Owner Author

karlb commented Jul 30, 2022

Thanks @bztsrc and @N-R-K!

@karlb karlb deleted the table branch July 30, 2022 08:10
@N-R-K
Copy link
Collaborator

N-R-K commented Jul 30, 2022

One more note, a lot of the other parsers allow the table to be indented 1 while our parser is strict about the table beginning at the start of the line surrounds the table with <p> .. </p> when the table is indented.

I think being strict is fine because it's simpler. So the current behavior is OK by me until/unless CommonMark has a specification for table syntax. Pointing it out just in case.

Footnotes

  1. https://babelmark.github.io/?text=++%7C+1st+field+%7C+2nd+field+%7C%0A++%7C+%3A--+++++++%7C+++%3A--%3A++++%7C%0A++%7C+1st+entry+%7C+2nd+entry+%7C%0A

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.

Does not support tables (patch included)
2 participants