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

Format markdown code for tables #3516

Closed
max-nextcloud opened this issue Dec 5, 2022 · 9 comments · Fixed by #4390
Closed

Format markdown code for tables #3516

max-nextcloud opened this issue Dec 5, 2022 · 9 comments · Fixed by #4390
Assignees
Labels

Comments

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Dec 5, 2022

This is a challenging good first issue Good for newcomers if you...

  • want to learn about how text serializes its content to markdown.
  • know your way around javascript including functional programming styles.

If you would like some guidance ping @max-nextcloud or join the public text channel.

Problem

When creating a table the resulting markdown source is not aligned.

Solution

Create pretty tables like https://www.tablesgenerator.com/markdown_tables

Currently tables are written to the markdown files like this:

| A | B | C |
|---|---|---|
| Columns | with long | text |

The goal of this issue would be to make sure the table is formatted in the markdown file like this:

| A       | B         |  C   |
|---------|-----------|------|
| Columns | with long | text |
Alternatives we considered
  • We could also try to preserve the format of existing tables. However that seems tedious as every cell could have it's own spacing and then editing the content might need to change that as well.
  • For some tables it may make more sense to drop alignment - for example if they include a single large cell that would spread the entire table over a multi screen width.
    For now we settled on creating nicely formatted tables. This could be changed in the future based on user feedback.

Starting points

Testing

The existing test for serializing tables loads a markdown input file, serializes it and checks the output is the same as the input.
The table in the markdown file is currently not aligned. If you change it to be nicely formatted the test will fail because text currently does not serialize the table content that way. So now you have a failing test you can start from.

Markdown rendering of tables

The toMarkdown functions in the table nodes define how to serialize the table to markdown.
Each file in that folder describes a node, which corresponds to an html element in the table. Rendering of the result starts with rendering the Table node, descends into to the TableHeader node, then further into the TableHeadRow and then TableCell and so on. Right now the TableCells just start with a leading whitespace and end with a trailing one. In order to format the table nicely the number of whitespaces will have to depend on the largest cell in a given column.

Computing the cell width

In order to compute the width of the columns the content of the entire table needs to be considered. The to markdown function gets arguments of the following form:

toMarkdown(state: MarkdownSerializerState, node: Node, parent: Node, index: number)

The node argument is a ProsemirrorNode and you can use descendants to iterate through all of the child nodes.
Starting from the Table itself, iterate through all of the children. We're only interested in the TableCells here. The function handed to descendants will get the child node as a first argument. If it's not a TableCell just return true (so descandents continues to look for TableCells inside the node in question. If it's a table cell you can use the index to identify the column this TableCell is in.

Providing the cell width to the TableCell toMarkdown function

Markdown serialization happens based on prosemirror-markdown. MarkdownSerializerState can be used to track the required width for each column.

Additional context

Strictly speaking this is not part of perserving md source - still came up when discussing #593 .

@max-nextcloud

This comment was marked as duplicate.

@max-nextcloud max-nextcloud added the good first issue Good for newcomers label Dec 5, 2022
@bronson

This comment was marked as resolved.

@max-nextcloud

This comment was marked as resolved.

@dacog

This comment was marked as resolved.

@max-nextcloud

This comment was marked as resolved.

@susnux

This comment was marked as resolved.

@max-nextcloud
Copy link
Collaborator Author

In order to make sure this good first issue Good for newcomers is easy to parse I tried to provide all information needed in the issue description above.

If some aspects of it are still unclear please ask so i can clarify further.

If you think this should be done differently please open a new issue describing your solution. Let's keep this issue focused on the proposed solution to provide clarity for whoever wants to tackle it.

@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team May 15, 2023
@luka-nextcloud
Copy link
Contributor

@max-nextcloud The markdown result of table already has the alignment.

image
image

@juliusknorr
Copy link
Member

juliusknorr commented Jun 21, 2023

I think there was a misunderstanding with the issue. It is about nicely formatting the actual markdown source code in.

Some example:

Currently tables are written to the file like this:

| A | B | C |
|---|---|---|
| Columns | with long | text |

The goal of this issue would be to make sure the table is formatted in the markdown file like this

| A       | B         |  C   |
|---------|-----------|------|
| Columns | with long | text |

(I've also put this example in the first post)

@luka-nextcloud luka-nextcloud moved this from 🧭 Planning evaluation (don't pick) to 🏗️ In progress in 📝 Office team Jun 26, 2023
@luka-nextcloud luka-nextcloud self-assigned this Jun 26, 2023
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 📝 Office team Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants