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

[WIP] Update showdown extensions #5167

Closed
wants to merge 2 commits into from
Closed

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Apr 20, 2015

WIP Notes:

  • This PR currently has 2 commits purely for review purposes, as the first commit reorders the tests to be clearer, but that would obscure the subsequent changes when squashed.
  • It also currently pulls showdown from a branch, again for testing/review purposes. It's easier to test out with a reference to a branch as I can easily update the branch with additional fixes.
  • When this has been reviewed I'll clean it up ready.

The majority of the work for this PR actually lives in the showdown fork

This PR represents a significant change to the way that extensions are processed in showdown. The key to it all is that instead of parsing out code blocks and the like from individual extensions (and doing it differently in each one), I've moved that logic into showdown itself so that each extension can hook into it in it's own way.

Additionally, I've added ~2000 lines of tests for the extensions, covering them individually as well as in the combined form of ghostgfm. Although this is very thoroughly tested, there are no guarantees I didn't miss a whacky combination of markdown. Suggestions please!

closes #5039, #5028, #4659, #4627, #4592, #1501

This PR pulls in the new version of showdown + adds and updates some tests to show what has changed

  • Lots of additional whitespaces caused by a bad implementation of GFM newlines have been removed
  • One fix with newlines + pre tags is now undone, but is now closer to all other markdown handlers

@ErisDS
Copy link
Member Author

ErisDS commented Apr 21, 2015

This PR is failing just on node v0.12 with the following error:

sh: 1: grunt: not found
npm ERR! Test failed. See above for more details.

Seeing as grunt-cli is supposed to be provided for all node environments, I'm kinda assuming this is a travis bug.

@dbalders
Copy link
Member

Did a little bit of testing and found some problems with some of the escaping.

screen shot 2015-04-21 at 9 06 27 am

The lists seem to escape correctly, but are formatted a little weird. I think the use case of someone escaping an entire list seems rare, but thought i would put it in there anyway.

The most important fixes are the strikethrough, the img link, and the regular link using <code>.

And the # headers using <code> are also not escaping after the first one.

@ErisDS
Copy link
Member Author

ErisDS commented Apr 21, 2015

I've been sat staring at that screenshot going.... Wut? For the past 20minutes. Taking the first line as the major example, that's exactly the crazy crap this PR is meant to fix... and I have a test which shows that it does:

Then it suddenly dawned on me.... I've only updated showdown on the server side, not on the client - if you publish the post - you should see COMPLETELY different results. Updating the PR now.

@ErisDS
Copy link
Member Author

ErisDS commented Apr 21, 2015

Ok, updated :) The markdown headers inside a code block being broken is a #wontfix because <code> is inline, and headers are block. Same happens if you put single ticks around the whole lot, but all of these do work:

@ErisDS ErisDS changed the title [WIP] Update showdown extensions Update showdown extensions May 1, 2015
ErisDS added 2 commits May 2, 2015 15:24
closes TryGhost#5039, TryGhost#5028, TryGhost#4659, TryGhost#4627, TryGhost#4592, TryGhost#1501

This PR pulls in the new version of showdown + adds and updates some tests to show what has changed

- Lots of additional whitespaces caused by a bad implementation of GFM newlines have been removed
- One fix with newlines + pre tags is now undone, but is now closer to all other markdown handlers
@ErisDS ErisDS changed the title Update showdown extensions [WIP] Update showdown extensions May 2, 2015
@ErisDS
Copy link
Member Author

ErisDS commented May 3, 2015

Found a horrific bug in this, working on it 😞

@ErisDS
Copy link
Member Author

ErisDS commented Oct 8, 2015

Will reopen this when I've got it done properly

@ErisDS ErisDS closed this Oct 8, 2015
@ErisDS ErisDS deleted the issue-5163 branch March 1, 2017 12:48
@ErisDS ErisDS restored the issue-5163 branch March 1, 2017 12:53
@ErisDS ErisDS deleted the issue-5163 branch March 1, 2017 12:58
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

Successfully merging this pull request may close these issues.

Github flavoured Markdown can't handle Github flavoured Markdown
2 participants