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

feat: tables support #2573

Merged
merged 10 commits into from Dec 7, 2023
Merged

feat: tables support #2573

merged 10 commits into from Dec 7, 2023

Conversation

shagr4th
Copy link
Contributor

@shagr4th shagr4th commented Dec 3, 2023

Column alignment is supported, as well as pipe escaping
Should fix Issue #1843

web/src/labs/marked/parser/Table.tsx Fixed Show resolved Hide resolved
@shagr4th shagr4th changed the title Tables support feat: tables support Dec 3, 2023
@jxpsert
Copy link
Contributor

jxpsert commented Dec 4, 2023

This is a much-needed feature. Checks seem to pass, except for the backend ones that haven't run and aren't relevant.
Merging this will greatly improve memos.

@shagr4th
Copy link
Contributor Author

shagr4th commented Dec 4, 2023

Actually this is not quite merge-ready, because using a regex is quite limiting to determine the validity of a markdown table expression
Right now, the regex I wrote mandates the use of a starting "|" at the beginning of the header (which is optional in many parsers), and does not check if the headers count and hyphens count is the same (I need to subclass a custom Regexp class and add theses additional checks) : this one could trigger false positives on text content not intended to be in a table.

Also, inserting a wrongly formatted table alongside other tables, breaks some of them (even if they have a correct syntax)

Anyway, it's a good start I think, I'll solve these parsing issues

@shagr4th
Copy link
Contributor Author

shagr4th commented Dec 7, 2023

Sorry for the multiple commits, it was more difficult than I thought. Right now, this PR pass most of the formatting tests found in the markdown-it test suite

The ones actually failing are related to backlash and backticks escaping (not supported right now in the parser), but this is not related to this table extension, so maybe for a future PR

Copy link
Collaborator

@boojack boojack left a comment

Choose a reason for hiding this comment

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

LGTM. The syntax of table is really hard to implement with regular. Thanks for your contribution.

@boojack boojack merged commit 787cf2a into usememos:main Dec 7, 2023
5 checks passed
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.

None yet

3 participants