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

tables are no longer recognized if a pipe is not put at the beginning #7858

Closed
e075e opened this issue Mar 1, 2023 · 9 comments
Closed

tables are no longer recognized if a pipe is not put at the beginning #7858

e075e opened this issue Mar 1, 2023 · 9 comments
Labels
bug It's a bug desktop All desktop platforms high High priority issues renderer About the note renderer upstream There's a problem with upstream code.

Comments

@e075e
Copy link

e075e commented Mar 1, 2023

Environment

Joplin version: 2.10.8
Platform: Android & Linux

Steps to reproduce

  1. create a note with a table, for instance:

column 1 | column 2
- | -
text 1 | text 2

Describe what you expected to happen

until version 2.9.17 this was recognized by the viewer as a table even if it does not have a pipe | at the beginning of every row.

Since version 2.10.8 a pipe must be present at least at the line that separate column's titles from rows, like this:

column 1 | column 2
| - | -
text 1 | text 2

As far as I know, the former was a "short way" to make a table in markdown, accepted also by other program (eg ReText), but I have no idea how much it's official

@e075e e075e added the bug It's a bug label Mar 1, 2023
@laurent22
Copy link
Owner

Are you sure it ever worked? Or maybe you had the table module enabled in the options? (Under the Markdown section)

@e075e
Copy link
Author

e075e commented Mar 1, 2023

100 % sure

I did not change options under the markdown section and "enable multimardown table extension" is set to false, but enabling it have the same behaviour as previous version. Probably, before the update to 2.10.8 that was enabled and disabled during/after the update

@Daeraxa
Copy link
Collaborator

Daeraxa commented Mar 1, 2023

The GFM spec says:

A leading and trailing pipe is also recommended for clarity of reading, and if there’s otherwise parsing ambiguity.

So it should still be supported without leading and trailing pipes.

This is 2.8.8 without the table markdown extension:

image

This is pulled from dev:
image

You can see that codemirror actually realises it is a valid table but the HTML render doesn't which seems like a regression somewhere.

@laurent22 laurent22 added desktop All desktop platforms high High priority issues renderer About the note renderer labels Mar 1, 2023
@laurent22
Copy link
Owner

Broken due to this upstream change: markdown-it/markdown-it#767

@laurent22 laurent22 added the upstream There's a problem with upstream code. label Mar 2, 2023
@tessus
Copy link
Collaborator

tessus commented Mar 14, 2023

It doesn't seem they are inclined to fix this upstream.

@tessus
Copy link
Collaborator

tessus commented Mar 14, 2023

I confirmed that on Linux it does not work.

On macOS it still seems to work:

Joplin 2.10.10 (dev, darwin)

Client ID: e354a932e6e143c782264aad4112b674
Sync Version: 3
Profile Version: 42
Keychain Supported: Yes

Revision: d48a5efa0 (dev)

image

@rlidwka
Copy link

rlidwka commented May 26, 2023

It doesn't seem they are inclined to fix this upstream.

I'm markdown-it maintainer, and I think I can answer what happened there.

Source of the issue

Github parses your code as a list, as demonstrated below:

a | b
- | -
1 | 2

a | b

  • | -
    1 | 2

This change in markdown-it is done to align markdown-it with cmark-gfm behavior. It seems to be much better if all parsers render your text the same way. But previously, if you copy-pasted your markdown into github, it'd just be broken.

Note that you don't have to have leading pipe, one extra minus would suffice:

column 1 | column 2
-- | --
text 1 | text 2

How to fix this

  1. I suggest to open an issue against https://github.com/github/cmark-gfm first, be sure to mention other parsers (ReText?) which behave differently. They are the maintainers of gfm spec, so it's their job to clarify it.
  2. If they change the spec or github parser, open a new issue in markdown-it to reflect those. Replying in an existing closed issue will easily get forgotten like it just happened with this case.

I believe the position of markdown-it team on this case is: we don't care how it is rendered. Just please make it behave the same way with all parsers.

@laurent22
Copy link
Owner

You don't seem to consider that people's documents are broken now while the "fixes" you propose would take months to get anywhere, and most likely nothing will change since they aren't going to change the spec for us.

I feel the way this was handled was careless to be honest. The spec says this:

A leading and trailing pipe is also recommended for clarity of reading, and if there’s otherwise parsing ambiguity.

So it's only a recommendation, and just for this you potentially broke the documents of hundreds of people. I hope Markdown-it won't make too many such changes in the future just for a supposed correctness.

@laurent22
Copy link
Owner

Closing as unfortunately there's not much not we can do about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug desktop All desktop platforms high High priority issues renderer About the note renderer upstream There's a problem with upstream code.
Projects
None yet
Development

No branches or pull requests

5 participants