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.

Sign up for GitHub

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

Jump to bottom

Fix/ Upgrade marked #808

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Fix/ Upgrade marked #808

merged 3 commits into from
Jan 27, 2022

Conversation

xkopenreview
Copy link
Collaborator

currently the forum page will render blank if any reply in the forum caused marked to throw an error

this pr added a try catch so that the forum can load

@xkopenreview xkopenreview linked an issue Jan 25, 2022 that may be closed by this pull request
Table followed by non-breaking space will break marked v3.0.0 and above #807
Closed
@xkopenreview
Copy link
Collaborator Author

to test: get a dump before today and go to
http://localhost:3030/forum?id=lRYfPNKCRu&noteId=SY78OiIyRi-

or use the example in #807

@zbialecki
Copy link
Contributor

It looks like the issue will be fixed in the next release of marked so this may not be needed: markedjs/marked#2332

In the future, if we want to prevent these errors my suggestion is that the note editor tests the note content before saving and shows the user an error so they can correct the markdown themselves.

@xkopenreview
Copy link
Collaborator Author

It looks like the issue will be fixed in the next release of marked so this may not be needed: markedjs/marked#2332

In the future, if we want to prevent these errors my suggestion is that the note editor tests the note content before saving and shows the user an error so they can correct the markdown themselves.

thanks for the link but i think the fix is not where it's failing in forum page.
so let's see if it will work in the next release

i think the difficult part is that we don't know how user would break marked.js and the error message are not helpful for user to fix the markdown themselves
this pr shows the error message in banner and preview tab if user use preview

@zbialecki
Copy link
Contributor

The new version of marked was released with PR #2332 merged: https://github.com/markedjs/marked/releases/tag/v4.0.11

Can you test it out and see if it fixes the issue?

@xkopenreview
Copy link
Collaborator Author

The new version of marked was released with PR #2332 merged: https://github.com/markedjs/marked/releases/tag/v4.0.11

Can you test it out and see if it fixes the issue?

no
image

do you want to make a pr to fix it?
seems they are quite fast merging prs

@zbialecki
Copy link
Contributor

We'll see... markedjs/marked#2372

@zbialecki
Copy link
Contributor

Wow, that was fast! https://github.com/markedjs/marked/releases/tag/v4.0.12

@xkopenreview
Copy link
Collaborator Author

Wow, that was fast! https://github.com/markedjs/marked/releases/tag/v4.0.12

great, i think it's working now

@zbialecki zbialecki changed the title Fix/ catch error in marked Fix/ Upgrade marked Jan 27, 2022
@zbialecki zbialecki merged commit e5af7e5 into master Jan 27, 2022
@zbialecki zbialecki deleted the fix/catch-marked-error branch January 27, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zbialecki zbialecki zbialecki approved these changes

@melisabok melisabok Awaiting requested review from melisabok

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Table followed by non-breaking space will break marked v3.0.0 and above
2 participants
@xkopenreview @zbialecki