Add syntax for tables #11

gpetiot · 2022-09-09T12:55:12Z

This is a first draft, I'm asking for feedback before going further. @jonludlam @Julow

I chose this syntax:

{t
  {tr
     {td ...}
     ...
     {td ...}
  }
  ...
  {tr
     {td ...}
     ...
     {td ...}
  }
}

The markup is ovsiously inspired from the <tr></tr><td></td> HTML tags.
The generated output is straightforward in HTML and Latex, at first I wanted to add header cells (<th></th>) but I'm not aware of anything similar in Latex but we could apply a different style.

Some improvements:

add the ability to say that a cell will span over N>1 columns. In HTML it would be <td colspan="N">...</td>, in Latex it would be \multicolumn{N}{c}{...}
check that each row has the same number of columns? (I don't remember if that will cause issues for Latex, but HTML should be more lenient).

What do you think?

jonludlam · 2022-09-09T13:25:32Z

Thanks @gpetiot !

This is definitely an approach that could work very well. However, I think we ought to consider the possibility of having more visual way to layout tables, perhaps something similar to how rst or orgmode does it - as an example both of the following are valid rst tables:

+------------+------------+-----------+
| Header 1   | Header 2   | Header 3  |
+============+============+===========+
| body row 1 | column 2   | column 3  |
+------------+------------+-----------+
| body row 2 | Cells may span columns.|
+------------+------------+-----------+
| body row 3 | Cells may  | - Cells   |
+------------+ span rows. | - contain |
| body row 4 |            | - blocks. |
+------------+------------+-----------+

and

=====  =====  ======
   Inputs     Output
------------  ------
  A      B    A or B
=====  =====  ======
False  False  False
True   False  True
False  True   True
True   True   True
=====  =====  ======

and here's an org-mode table:

| N | N^2 | N^3 | N^4 | sqrt(n) | sqrt[4](N) |
|---+-----+-----+-----+---------+------------|
| / |  <  |     |  >  |       < |          > |
| 1 |  1  |  1  |  1  |       1 |          1 |
| 2 |  4  |  8  | 16  |  1.4142 |     1.1892 |
| 3 |  9  | 27  | 81  |  1.7321 |     1.3161 |
|---+-----+-----+-----+---------+------------|

The reason being that more often than not right now I'm still looking at docs in my editor buffer, or on github, as opposed to nicely rendered on a web page. We'd also get to use pre-existing table-editing modes in various editors.

Julow · 2022-09-09T13:28:22Z

Features that other languages have: text alignment of the column, header rows.

The short syntax from RST is interesting: https://docutils.sourceforge.io/docs/ref/rst/directives.html#list-table if we drop the structured directive and attributes and make it less ambiguous, we get a syntax easy to type that is similar to lists and might be enough in most cases.

I suggest this syntax, which works the same way as lists:

++ - header 1
   - header 2
** - cell 1
   - cell 2

The long syntax that you suggest is needed, like we have for lists, to support nesting of bigger elements.

To leave room for the colspan feature and text alignment, we need some kind of attributes: {td[colspan=2,align=center] ...} (similar to CSS selectors).

Header row similar to HTML:

{table
 {tr
  {th header 1}
  {th header 2}}
 {tr
  {td cell 1}
  {td cell 2}}}

The main element can be {table ...}, it's more readable and similar to HTML.

dbuenzli · 2022-09-09T13:30:36Z

Most lightweight markup languages add two things, headers and specification of column alignement (see for example the github flavoured markdown table definition). I think it would be nice to have these. I wouldn't bother too much with multi-columns.

I have mixed feelings about what @jonludlam suggests. Of course these tables look nicer but the given examples are also not very content heavy.

Once you start adding links, references and ocamldoc markup in the cell contents I have the impression that the regular, well-delimited, syntax of ocamldoc scales better.

Julow · 2022-09-09T13:40:13Z

The reason being that more often than not right now I'm still looking at docs in my editor buffer, or on github, as opposed to nicely rendered on a web page.

I've always hated both reading and writing tables drawn and aligned like that. The editor plugins to draw them are not worth learning for the few tables I ever needed to make so I end up not aligning them, they become write-only. Also, multi line cells are a pain.

jonludlam · 2022-09-09T13:45:09Z

@dbuenzli agreed that larger-scale tables won't look as nice as those examples! Nothing stopping us from doing both though - we already have precedent in lists, where we can write both

(** Here is a {b list}
- item 1
- item 2
- item 3

The list is ended by the blank line.*)

and

(** Here is a {b list}
{ul {- item 1}
{- item 2}
{- item 3}}
The list is ended by the blank line.*)

we could simply choose based on the content of the {table ...} element - e.g.

{table
 {tr
  {th header 1}
  {th header 2}}
 {tr
  {td cell 1}
  {td cell 2}}}

vs

{table
+==========+==========+
| header 1 | header 2 |
+==========+==========+
| cell 1   | cell 2   |
+----------+----------+
}

jonludlam · 2022-09-09T13:46:22Z

I've always hated both reading and writing tables drawn and aligned like that. The editor plugins to draw them are not worth learning for the few tables I ever needed to make so I end up not aligning them, they become write-only. Also, multi line cells are a pain.

I've never hated reading them, but writing is a bit of a pain without editor support :-)

jonludlam · 2022-09-09T13:59:45Z

Actually, choosing based on contents is not a good idea. We could use a different tag though - {t vs {table for example.

dbuenzli · 2022-09-09T14:00:28Z

To motivate with concrete examples here's a sample of tables that I currently write as code and that would benefit from the proposal:

Scroll a bit if you don't see the table, which reminds me: it would be nice to be able to reference them via a label like we do for sections, i.e. {table:label (and why not cells and/or rows, but let's start simple).

dbuenzli · 2022-09-09T14:13:11Z

Regarding a possible lightweight notation. I think it's good at least have a look at what the experts do to when they can start designing from scratch.

gpetiot · 2022-09-09T17:36:37Z

For the light syntax, what do you think about something like:

{t
+-----------------------------
|{l}  xx |{c}  yy  |{r}   zz
+---------------------------
|   xx   |{r}   yy  |{l}    zz
+-----------------------------
}

No need to finish the line (just +- at the beginning of the line to indicate a new row)
{l}/{r}/{c} could indicate the alignment (optional), but should be stuck to the | to be a special token

dbuenzli · 2022-09-09T18:24:29Z

For the light syntax, what do you think about something like:

If you really want to add this then I strongly suggest not to invent anything new here. Have a look at pandoc's markdown table extensions for a few alternatives.

Personally I think you should just go with pipe_tables which is popular among a few extended markdown dialects including github's and gitlab's ones.

gpetiot · 2022-09-19T10:06:18Z

I implemented the light and heavy syntaxes, this PR is ready to be reviewed.

Julow

This parses as two columns, a and b |c| d, which is not intuitive:

{t
 | a | b |c| d |
 |---|--:|:--|:-:|
}

src/syntax.ml

jonludlam · 2022-09-20T08:38:36Z

I assume that, since pipetables are pretty standard, there are a set of nasty test cases somewhere that we could use?

dbuenzli · 2022-09-20T10:01:21Z

Would it please be possible to update the first comment with the implemented syntaxes. For example how do I specify alignement in the "heavy" syntax ?

gpetiot · 2022-09-20T18:36:21Z

Would it please be possible to update the first comment with the implemented syntaxes. For example how do I specify alignement in the "heavy" syntax ?

Nothing decided yet, I'm torn between doing something similar to the light syntax:

{table
   {tr {th ...} [th ...}}
   {align left right}
   ...
}

or something that would be easier to read to my opinion but that would make both implementations diverge:

{table
  {tr {th{left} ...} {th{right} ...}} or {tr {th:left ...} {th:right ...}}
  ...
}

dbuenzli · 2022-09-20T18:56:24Z

Note that even though IIRC odoc dropped support for it. There was a syntax in ocamldoc for text alignement. Might be worth resurecting it for th. Something like: a bare th is left-aligned (or whathever the default is of the light syntax) and otherwise alignement is specified by "direct" nesting:

{tr
{th {L Left aligned}} {th {R Right aligned}} {th {C Center aligned}} {th default align}}

gpetiot · 2022-09-21T14:53:53Z

Note that even though IIRC odoc dropped support for it. There was a syntax in ocamldoc for text alignement. Might be worth resurecting it for th. Something like: a bare th is left-aligned (or whathever the default is of the light syntax) and otherwise alignement is specified by "direct" nesting:
{tr
{th {L Left aligned}} {th {R Right aligned}} {th {C Center aligned}} {th default align}}

Sounds good to me, @jonludlam what do you think?

jonludlam · 2022-09-21T14:56:14Z

Reusing the syntax we previously had makes sense to me.

Julow · 2022-09-21T15:55:56Z

On the other hand, an attribute-like syntax would allow colspan and keep room for new things later.

Julow

I'm not a fan of the pipe syntax at all but using the most common syntax can't hurt Odoc.

src/compat.ml

src/syntax.ml

src/compat.ml

gpetiot · 2022-12-12T20:45:02Z

I fixed the parsing issues with the alignment tags for the heavy syntax. Let me know what you think @Julow @jonludlam

Julow

Nice!

Adding a block element into a light table makes quite a mess of warnings and continue the parsing in an inconsistent way:

# p {|
  {t
   | {[ a ]} | b |
   |---|---|
  }
  |} ;;
File "", line 2, characters 5-12:
'{[...]}' (code block) is not allowed in '{t ...}' (table).
File "", line 2, characters 13-14:
'|' is not allowed in top-level text.
Suggestion: move '|' into '{t ...}' (table).
File "", line 2, characters 15-16:
Paragraph should begin on its own line.
File "", line 2, characters 17-18:
'|' is not allowed in top-level text.
Suggestion: move '|' into '{t ...}' (table).
File "", line 3, characters 3-4:
'|' is not allowed in top-level text.
Suggestion: move '|' into '{t ...}' (table).
File "", line 3, characters 7-8:
'|' is not allowed in top-level text.
Suggestion: move '|' into '{t ...}' (table).
File "", line 3, characters 8-11:
Paragraph should begin on its own line.
File "", line 3, characters 11-12:
'|' is not allowed in top-level text.
Suggestion: move '|' into '{t ...}' (table).
File "", line 4, characters 2-3:
Unpaired '}' (end of markup).
Suggestion: try '\}'.
- : Ast.t =
[`Table (Light {header = []; data = []; align = []});
 `Paragraph [`Word "b"; `Space " "]; `Paragraph [`Word "---"];
 `Paragraph [`Word "---"]; `Paragraph [`Word "}"]]

The parser has the goal of not loosing text in case of bad syntax so the problematic block element shouldn't be removed. It would be nice to handle the conversion from table to paragraphs without generating too much warnings.

src/ast.ml

src/syntax.ml

panglesd

Nice !

I do have change requested for some things:

I think that the heavy syntax should allow us to define any cell to be a header cell (just like in html). This allows for several use cases, with two common one being: "vertical" tables (tables where the header is a column) and tables with two "axis" (both a header line and a header column).
Fortunately, I think this will actually simplify the code a lot.
I believe the lexer should do more work in the case of the light table. Currently, as mentioned by @Julow, opening a forbidden block inside a light table will generate a lot of warnings (where one would have sufficed) and the next closing will be wrongly paired. That's a major issue!
Using a specific lexer for the inside of light tables (which is what is done for verbatim for example) also removes the constraint added by this PR to escape | characters outside of tables.
I don't think the AST type should make such a clear distinction between light and heavy syntax. Similarly to `` List, a variant on the side could distinguish between them, the information is almost always irrelevant for consumers of the parser, it has to be easy to ignore.

Another thing: I am not a huge fan of the alignment syntax for heavy tables.
To me, it seems that {th {R text}} will only have an effect on the cell, by the nesting. (The fact that it has an effect on the whole columns can only be understood with the light syntax in mind).
Moreover, it prevents possible future uses of {L } to specify the alignment on single cell, especially on header cell (if we want that, not sure...).
Instead, we could take inspiration from latex's table (or the col html element), where the fact that it acts on columns is evident:

{table {col L C R}
  {tr
    {th left-aligned} {th centered} {th right-aligned}}
  }
}

This leaves room for deciding the line borders in odoc (but maybe that's not something we want):

{table {col | L C | R}
  {hline}
  {tr
    {th left-aligned} {th centered} {th right-aligned}}
  }
  {hline}
}

src/syntax.ml

src/compat.ml

src/ast.ml

New lines inside markup (such as `{e ...}`) were not detected, although it breaks the light table format. Add a warning for such cases. Signed-off-by: Paul-Elliot <peada@free.fr>

Signed-off-by: Paul-Elliot <peada@free.fr>

contribution to the table PR

jonludlam · 2023-02-13T22:54:38Z

I had a quick look to see whether there were any differences running over the set of libraries in odoc's driver with the ODOC_BENCHMARK var set to true. I found a two differences - here:

In both instances they are actually code snippets without being enclosed in any sort of verbatim/code tags, but I was slightly surprised nevertheless. It's not a show stopper but I'd be quite interested to know why the differences are there.

Signed-off-by: Paul-Elliot <peada@free.fr>

Fix new paragraph starting on `|`

This commit fixes several alignment issues. 1. No alignment in a cell was encoded as an alignment to `Center. "Centering" was certainly a typo, but there is also the issue that "no specified alignment" should really be different than "specified to ..." For instance (assuming `{R ...}` exists): ``` {R {t |aaa|bbb| |---|---| |xxx|yyy|}} ``` would not be right-aligned inside the table, with the previous behaviour. This commit allows for "unspecified alignment" using an option. 2. No alignment in a table was encoded as an empty list of alignment. This was wrong, since at some point we might want to raise warnings in case the number of alignment and the number of columns differ. Signed-off-by: Paul-Elliot <peada@free.fr>

Fix default alignment

panglesd · 2023-02-20T11:53:34Z

Thanks @jonludlam for spotting a bug! It has been fixed in 97fd2a3.

I've updated ocaml/odoc#893 to be in sync with this PR, in case you want to review them as a whole.

jonludlam · 2023-02-22T18:01:03Z

src/syntax.ml

@@ -209,9 +324,11 @@ and delimited_inline_element_list :
    parent_markup:[< Token.t ] ->
    parent_markup_location:Loc.span ->
    requires_leading_whitespace:bool ->
+    context:inline_context ->


This bit of threaded state is annoying. It's also very similar to the requires_leading_whitespace state - I wonder if the two could be combined into a single Context.t or similar? I suppose the new bit is relevant to both delimited_inline_element_list and inline_element so maybe not worth it? It's also possibly worth considering getting rid of the context and just checking in the light_table code after we've called this function that the start and end lines are the same?

I had a quick go: jonludlam@5c49f98 - I think it's a bit less invasive and keeps the check within the light_table_row function. What do you think?

It's also my opinion that having the check in light_table_row makes the code clearer.

As a small remark, the location warning in the current PR and jonludlam@5c49f98 will be different:

In the current PR, it is only the newline. There is one warning per newline.

In jonludlam@5c49f98 there is only one warning, with location being the whole inline item.

I think both solutions have different advantages/disadvantages (one is clearer, the other more general), and I don't have a preference. So let's go with @jonludlam's idea!

jonludlam · 2023-02-27T16:35:10Z

Looking good now, thanks @gpetiot!

jonludlam mentioned this pull request Sep 9, 2022

Add support for tables ocaml/odoc#864

Closed

gpetiot mentioned this pull request Sep 19, 2022

Add support for tables ocaml/odoc#893

Merged

Julow reviewed Sep 20, 2022

View reviewed changes

src/syntax.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

MisterDA mentioned this pull request Oct 12, 2022

Convert manual from Wiki to mld ocsigen/lwt#951

Merged

Julow reviewed Oct 20, 2022

View reviewed changes

src/compat.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

src/compat.ml Outdated Show resolved Hide resolved

src/compat.ml Outdated Show resolved Hide resolved

gpetiot requested a review from Julow December 12, 2022 20:45

Julow reviewed Jan 13, 2023

View reviewed changes

src/ast.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

gpetiot requested a review from Julow January 17, 2023 15:50

panglesd reviewed Jan 17, 2023

View reviewed changes

src/syntax.ml Outdated Show resolved Hide resolved

src/syntax.ml Outdated Show resolved Hide resolved

Add syntax for tables

82af0c3

panglesd suggested changes Feb 1, 2023

View reviewed changes

gpetiot added 6 commits February 7, 2023 13:29

Replace let>> with >>>

94bddbd

Simplify alignment

cd1f507

join_list without exception

bca4972

Convert light table contents to nestable block elements

0e7e528

Do not allow alignment in heavy tables

6237d0f

Typo

9c34050

MisterDA mentioned this pull request Feb 10, 2023

Generate opam file, dune.3, omd.2.0.0~alpha3, emit tables! mseri/md2mld#15

Merged

gpetiot and others added 7 commits February 10, 2023 16:08

Preserve td/th in the datatype

486fd61

Renaming

e3738d0

Warn new lines in markup nested in light tables

36d381a

New lines inside markup (such as `{e ...}`) were not detected, although it breaks the light table format. Add a warning for such cases. Signed-off-by: Paul-Elliot <peada@free.fr>

Add suggestion on wrong content inside {table } and {tr }`

bdfeef1

Signed-off-by: Paul-Elliot <peada@free.fr>

allow | char as a word outside of light table

bd19b75

Signed-off-by: Paul-Elliot <peada@free.fr>

adding a test for varying number of cells

d72b51c

Signed-off-by: Paul-Elliot <peada@free.fr>

Merge pull request #1 from panglesd/tables

55a4197

contribution to the table PR

panglesd and others added 4 commits February 14, 2023 11:33

Fix new paragraph starting on |

97fd2a3

Signed-off-by: Paul-Elliot <peada@free.fr>

Merge pull request #2 from panglesd/tables

d20885a

Fix new paragraph starting on `|`

Merge pull request #3 from panglesd/tables

497fe75

Fix default alignment

gpetiot added 3 commits February 20, 2023 17:49

Handle '+' to separate columns in 'align' row

1199bd4

Renaming data->grid in tests, data is a kind of a cell

53d2161

Revert 1199bd4

a6f0526

jonludlam reviewed Feb 22, 2023

View reviewed changes

Alternate method of invalid light table syntax

86583cc

jonludlam merged commit 370f1cf into ocaml-doc:main Feb 27, 2023

gpetiot deleted the tables branch February 27, 2023 16:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add syntax for tables #11

Add syntax for tables #11

gpetiot commented Sep 9, 2022

jonludlam commented Sep 9, 2022

Julow commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

Julow commented Sep 9, 2022

jonludlam commented Sep 9, 2022

jonludlam commented Sep 9, 2022

jonludlam commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

gpetiot commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

gpetiot commented Sep 19, 2022

Julow left a comment

jonludlam commented Sep 20, 2022

dbuenzli commented Sep 20, 2022

gpetiot commented Sep 20, 2022

dbuenzli commented Sep 20, 2022

gpetiot commented Sep 21, 2022

jonludlam commented Sep 21, 2022

Julow commented Sep 21, 2022

Julow left a comment

gpetiot commented Dec 12, 2022 •

edited

Loading

Julow left a comment

panglesd left a comment •

edited

Loading

jonludlam commented Feb 13, 2023

panglesd commented Feb 20, 2023

jonludlam Feb 22, 2023

jonludlam Feb 22, 2023

gpetiot Feb 23, 2023

panglesd Feb 23, 2023

jonludlam commented Feb 27, 2023

Add syntax for tables #11

Add syntax for tables #11

Conversation

gpetiot commented Sep 9, 2022

jonludlam commented Sep 9, 2022

Julow commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

Julow commented Sep 9, 2022

jonludlam commented Sep 9, 2022

jonludlam commented Sep 9, 2022

jonludlam commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

gpetiot commented Sep 9, 2022

dbuenzli commented Sep 9, 2022

gpetiot commented Sep 19, 2022

Julow left a comment

Choose a reason for hiding this comment

jonludlam commented Sep 20, 2022

dbuenzli commented Sep 20, 2022

gpetiot commented Sep 20, 2022

dbuenzli commented Sep 20, 2022

gpetiot commented Sep 21, 2022

jonludlam commented Sep 21, 2022

Julow commented Sep 21, 2022

Julow left a comment

Choose a reason for hiding this comment

gpetiot commented Dec 12, 2022 • edited Loading

Julow left a comment

Choose a reason for hiding this comment

panglesd left a comment • edited Loading

Choose a reason for hiding this comment

jonludlam commented Feb 13, 2023

panglesd commented Feb 20, 2023

jonludlam Feb 22, 2023

Choose a reason for hiding this comment

jonludlam Feb 22, 2023

Choose a reason for hiding this comment

gpetiot Feb 23, 2023

Choose a reason for hiding this comment

panglesd Feb 23, 2023

Choose a reason for hiding this comment

jonludlam commented Feb 27, 2023

gpetiot commented Dec 12, 2022 •

edited

Loading

panglesd left a comment •

edited

Loading