Fix pre issue + update marked.js #732

marcoscaceres · 2016-05-03T09:42:36Z

No description provided.

marcoscaceres · 2016-05-03T10:19:44Z

@tobie, git blame says this is mostly your baby. Hopefully I didn't break it. Would you mind having a looksy.

marcoscaceres · 2016-05-03T10:20:23Z

js/core/markdown.js

-            gfm: false,
-            pedantic: false,
-            sanitize: false
+            sanitize: false,


the ones I removed default to either true or false.

tobie · 2016-05-03T10:27:09Z

Your best bet is to compare https://coremob.github.io/coremob-2012/ with both scripts.

marcoscaceres · 2016-05-03T10:41:23Z

@tobie, ah, super helpful... some breakage 😢

halindrome · 2016-05-03T10:41:46Z

I am concerned about a comment you made in the spec-prod email about IDs on section elements no longer being provided. I wonder about the A11Y impact of that on screen readers and navigation. Also on other parts of ReSpec or plugins that might expect sections to have IDs. @LJWatson can you provide any insight on the A11Y front?

marcoscaceres · 2016-05-03T10:43:11Z

Also on other parts of ReSpec or plugins that might expect sections to have IDs

Very few people use markdown, afaict.

halindrome · 2016-05-03T10:46:27Z

I am sure you are right. I had forgotten it was supported.

marcoscaceres · 2016-05-03T10:53:06Z

Specs I've found - mostly by @igrigorik:

marcoscaceres · 2016-05-03T10:54:50Z

Found via:
https://github.com/search?l=html&p=1&q=org%3Aw3c+format+markdown&type=Code

Will check those tomorrow and see what I broke with Core Mob.

marcoscaceres · 2016-05-03T11:07:43Z

hmm... seems I need to convert all HTML nodes except section into text, then parse them as markdown. should be easy to do... famous last words.

marcoscaceres · 2016-05-05T09:15:22Z

Ok, this seems happy

marcoscaceres · 2016-05-05T09:20:49Z

And it passes!!!! 💃 woot woot!

tobie · 2016-05-05T09:33:08Z

js/core/markdown.js

+ *         <h3>Another subtitle</h3>
+ *         <p>More text.</p>
+ *       </section>
+ *     </section>


The // to /* ... */ style change makes it super hard to figure out if the content changed or not.

True. Sorry about that.

marcoscaceres · 2016-05-05T09:42:03Z

I've checked the spec list also. They all seem to parse correctly.

marcoscaceres · 2016-05-05T09:43:02Z

@tobie, maybe you could give it a once over? let me know if it looks ok to merge.

tobie · 2016-05-05T09:43:10Z

js/core/markdown.js

-define(
-    ['core/marked'],
-    function (coreMarked) {
-        marked.setOptions({


So there's something fishy going on here with regards to the global marked object and the module coreMarked object. My hunch is that I had reasons to do this. I'm not sure why I didn't document them, though. It could also just be a refactoring mistake that I failed to notice because I was then hitting the global marked object.

You might want to check whether coreMarked and marked now point to the same thing.

Will check. Remember I also updated marked... so there should not be a global (coreMarked).

Checked. Marked no longer leaks into global scope.

marcoscaceres · 2016-05-05T09:47:43Z

@tobie I only did very minor refactor of makeBuilder()...I couldn't figure it out... was super fancy impressive tho :) The rest I rewrote and chopped out a lot of code.

tobie · 2016-05-05T09:53:17Z

I only did very minor refactor of makeBuilder()...I couldn't figure it out... was super fancy impressive tho :) The rest I rewrote and chopped out a lot of code.

I wrote this a while back, now, so memory fails me, but iirc, the reason for this is that the rest of the pipeline expects nested sections, so you basically have to recreate the nesting from the flat H1-6 hierarchy that the markdown parser provides.

tobie · 2016-05-05T09:56:51Z

js/core/utils.js

+                if (typeof text !== "string") {
+                    throw TypeError("Invalid input");
+                }
+                var hasOpeningPre = testRegex(/<pre\b[^>]*>/i);


Conceptually hostile to the idea of recreating your own mini HTML parser using regexes here given you're in a browser and can leverage the native one. Maybe that's the onlu sane solution, though.

I'll give this some thought... I could work with a live DOM, and check if .parentElement is <pre>

tobie · 2016-05-05T10:09:50Z

Alright--done with the questions/comments here.

marcoscaceres · 2016-05-05T11:31:05Z

Thanks @tobie! Great feedback. Will fix up!

I'm wanting to add language-based syntax highlighting next via highlight.js

Moar goodies coming soon
🍬🍦🍭🍩🍪🍡🍢🍮🍧

5 May 2016, at 8:09 PM, Tobie Langel notifications@github.com wrote:

Alright--done with the questions/comments here.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

marcoscaceres · 2016-05-06T05:56:51Z

@tobie, I rewrote it to use a HTML parser as you suggested. You were right: using regex for parsing HTML was a disgrace.

* develop: v3.2.122 Fix pre issue + update marked.js (#732) Chore(TravisCI): Test against Node 6 chore(package): update karma-mocha-reporter to version 2.0.3 (#740) chore(package): update nightmare to version 2.4.0 (#741) chore(package): update karma-safari-launcher to version 1.0.0 (#739) chore(package): update karma-firefox-launcher to version 1.0.0 (#738) chore(package): update karma-opera-launcher to version 1.0.0 (#737) chore(package): update karma-jasmine to version 1.0.2 (#736) chore(package): update karma-jasmine to version 1.0.0 (#734) chore(package): update karma-requirejs to version 1.0.0 (#733) chore(package): update karma-ie-launcher to version 1.0.0 (#735)

marcoscaceres force-pushed the update_marked branch 2 times, most recently from 973bece to efaf538 Compare May 3, 2016 09:59

marcoscaceres reviewed May 3, 2016
View reviewed changes

marcoscaceres force-pushed the update_marked branch 3 times, most recently from d471db2 to ea80498 Compare May 5, 2016 09:13

marcoscaceres added the needs review label May 5, 2016

tobie reviewed May 5, 2016
View reviewed changes

marcoscaceres force-pushed the update_marked branch from ea80498 to 5b41c6f Compare May 5, 2016 09:35

tobie reviewed May 5, 2016
View reviewed changes

marcoscaceres added 5 commits May 6, 2016 15:47

Fix(markdown) stop stripping whitespace from pre (closes #724)

742b218

Chore(marked): Upgrade marked.js (closes #581)

e84d284

Feat(utils): teach utils how to normalize padding

0828b1f

Chore(markdown-spec): add additional tests

ad73f06

Fix(ui-spec): Race condition in Safari

13cb89e

marcoscaceres force-pushed the update_marked branch from 5b41c6f to 5705209 Compare May 6, 2016 05:52

marcoscaceres added 3 commits May 6, 2016 16:09

Chore(markdown): refactor processing order

8ae715c

Feat(utils): normalize padding using html parser

93de965

Chore(markdown-spec): fixes based on @tobie feedback

d8f5944

marcoscaceres force-pushed the update_marked branch from 5705209 to d8f5944 Compare May 6, 2016 06:09

marcoscaceres added 2 commits May 6, 2016 16:20

Fix(package): enable linting markdown.js

3bb9b31

Chore(markdown): Make jscs happy

6434558

marcoscaceres merged commit 858f5b4 into develop May 6, 2016

marcoscaceres deleted the update_marked branch May 6, 2016 06:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix pre issue + update marked.js #732

Fix pre issue + update marked.js #732

marcoscaceres commented May 3, 2016

marcoscaceres commented May 3, 2016

marcoscaceres May 3, 2016

tobie commented May 3, 2016

marcoscaceres commented May 3, 2016

halindrome commented May 3, 2016

marcoscaceres commented May 3, 2016

halindrome commented May 3, 2016

marcoscaceres commented May 3, 2016 •

edited

marcoscaceres commented May 3, 2016

marcoscaceres commented May 3, 2016

marcoscaceres commented May 5, 2016

marcoscaceres commented May 5, 2016

tobie May 5, 2016

marcoscaceres May 5, 2016

marcoscaceres commented May 5, 2016

marcoscaceres commented May 5, 2016

tobie May 5, 2016 •

edited

marcoscaceres May 5, 2016

marcoscaceres May 6, 2016

marcoscaceres commented May 5, 2016

tobie commented May 5, 2016

tobie May 5, 2016

marcoscaceres May 6, 2016

tobie commented May 5, 2016

marcoscaceres commented May 5, 2016

marcoscaceres commented May 6, 2016 •

edited

Fix pre issue + update marked.js #732

Fix pre issue + update marked.js #732

Conversation

marcoscaceres commented May 3, 2016

marcoscaceres commented May 3, 2016

marcoscaceres May 3, 2016

Choose a reason for hiding this comment

tobie commented May 3, 2016

marcoscaceres commented May 3, 2016

halindrome commented May 3, 2016

marcoscaceres commented May 3, 2016

halindrome commented May 3, 2016

marcoscaceres commented May 3, 2016 • edited

marcoscaceres commented May 3, 2016

marcoscaceres commented May 3, 2016

marcoscaceres commented May 5, 2016

marcoscaceres commented May 5, 2016

tobie May 5, 2016

Choose a reason for hiding this comment

marcoscaceres May 5, 2016

Choose a reason for hiding this comment

marcoscaceres commented May 5, 2016

marcoscaceres commented May 5, 2016

tobie May 5, 2016 • edited

Choose a reason for hiding this comment

marcoscaceres May 5, 2016

Choose a reason for hiding this comment

marcoscaceres May 6, 2016

Choose a reason for hiding this comment

marcoscaceres commented May 5, 2016

tobie commented May 5, 2016

tobie May 5, 2016

Choose a reason for hiding this comment

marcoscaceres May 6, 2016

Choose a reason for hiding this comment

tobie commented May 5, 2016

marcoscaceres commented May 5, 2016

marcoscaceres commented May 6, 2016 • edited

marcoscaceres commented May 3, 2016 •

edited

tobie May 5, 2016 •

edited

marcoscaceres commented May 6, 2016 •

edited