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

Implementation of the table element #6

Open
2 of 6 tasks
mortenpi opened this issue May 17, 2022 · 2 comments
Open
2 of 6 tasks

Implementation of the table element #6

mortenpi opened this issue May 17, 2022 · 2 comments

Comments

@mortenpi
Copy link
Member

mortenpi commented May 17, 2022

The precise implementation of the Table element in CommonMark is currently unclear to me (not sure how the different sub-elements should be organized).

  • The Table extension needs to be properly hashed out and documented.
  • The conversion from standard library tables also needs to be fixed.
  • Remove the TableBody and TableHeader elements altogether, and just interpret the first row as the header.
  • Make TableCell a singleton, removing its fields. They could also be turned into some sort of dynamic properties that are determined by traversing the tree.
  • Make TableComponent subtype AbstractElement directly and don't use it for Table (to ensure that the internal nodes of a table are not allowed to exists in other contexts).
  • Add helper methods like rows, nrows, ncols etc. to dynamically determine the number of columns and rows of a table.
@mortenpi
Copy link
Member Author

mortenpi commented Jul 5, 2022

A few notes here:

  • cmark-gfm tables do not have the TableHeader and TableBody nodes.
  • Arguably, TableHeader and TableBody are pretty redundant and make things more complicated. Instead, we could just have TableRows as the children of Table directly, with the first TableRow implicitly interpreted as the header.
  • Currently, TableComponent is an AbstractBlock. However, you probably shouldn't use the various table components outside of a Table, so it might make sense to make Table <: AbstractBlock directly and all the other table components <: TableComponent <: AbstractElement.
  • If we allow the table to be mutated, then we might end up with different number of TableCells in each row. To make things consistent, we should say that the number of columns of a Table is the longest row and shorter rows are implicitly padded with empty TableCells.

@mortenpi
Copy link
Member Author

mortenpi commented Jul 5, 2022

A few additional notes:

  • CommonMark.jl defines TablePipe <: AbstractInline, and does construct the corresponding Nodes. However, it looks like this is just a temporary helper node for parsing and does not end up in the final AST..?
  • It would be helpful to have a rows(::Node) function that, when called on a Table node, returns e.g. header :: TableRow, bodyrows :: Vector{TableRow} that the user can loop over when working with a Table. It would take into account how to properly interpret a bad table, e.g. by merging TableBodys etc.

mortenpi added a commit that referenced this issue Jul 5, 2022
mortenpi added a commit that referenced this issue Aug 30, 2022
These are helper functions for working with tables that try to work
around some of the complexity the current element structure brings (see
also #6).
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

No branches or pull requests

1 participant