microlight icon indicating copy to clipboard operation
microlight copied to clipboard

Added method for direct text-to-HTML conversion

Open buu700 opened this issue 8 years ago • 11 comments

microlight.reset still exists for automatically traversing the DOM and performing in-place highlighting, but the core logic has been factored out as microlight.process for performing the same operation on a string input without side effects.

buu700 avatar May 31 '16 04:05 buu700

Also, it just occurred to me, I probably should've included this to begin with; here's a diff with indentation removed to make it more clear what actually changed: microlight pr diff.pdf

buu700 avatar May 31 '16 16:05 buu700

@buu700 You can display a diff without whitespace changes with the ?w=1 argument, e.g. for this pull request: https://github.com/asvd/microlight/pull/1/files?w=1

See also https://github.com/blog/967-github-secrets

P.S.: Really nice, minimal library @asvd . :)

blueimp avatar Jun 01 '16 13:06 blueimp

@buu700 thanks for your suggestion.

I would not merge your changeset, since I don't see a real reason for highlighting a text virtually (yet I am curious to know your thoughts on how this can be used). And besides this would increase the library size (I'm trying to keep it as minimal as possible mainly for fun).

By the way: you don't need to create a DIV element which is only used for storing the HTML content. Just put it into a string and then return.

asvd avatar Jun 01 '16 13:06 asvd

Nice, thanks for the tip @blueimp! That feature should really be way more prominent.

As far as the use case @asvd, this is how I would be using it at least: https://github.com/cyph/cyph/blob/next/shared/js/cyph/ui/directives/markdown.ts#L31. I'm not sure how common that type of usage would be, but it seems like a standard enough pattern (and addresses the complaint someone on HN had about it directly modifying the DOM).

As far as the size, are you specifically referring to the increase from 2.2kb to 2.3kb, or just making the more general point that you'll want to be conservative in what you add? FWIW, the gzipped size remains at 1.4kb with or without this change.

Got it, thanks for the clarification on the createElement thing. (I hadn't actually looked closely enough at the code to determine whether that was necessary; was mostly trying to make as few changes as possible.) I'll update this PR with that fixed in a bit.

buu700 avatar Jun 01 '16 15:06 buu700

I'm just preparing a more extended version of microlight, which will support editable areas, and also include some performance tweaks. So I can consider your suggestion to be added there, but not into microlight.js which I would like to keep in a minimal state.

What happens then in your project with highlighted code? Where do you put it then? Do you want to perform the highlight on server side? Because in this case the formatted text can become bigger than the library itself, as discussed in this branch on HN: https://news.ycombinator.com/item?id=11803389

asvd avatar Jun 01 '16 15:06 asvd

Fair enough, I'll just maintain this version as a separate fork until the extended version is available.

Ah sorry, I should clarify more specifically what we're actually doing. @cyph currently includes highlight.js in production for allowing users to send snippets of code to each other, which can only be done client-side because the context is user-generated content within a secure/E2EE chat.

screen shot 2016-06-01 at 11 37 19 am

That currently looks pretty bad because we hadn't yet gotten around to setting up a good custom style, but Microlight out of the box turned out to be great (in addition to the reduced footprint):

screen shot 2016-05-31 at 12 50 42 am

buu700 avatar Jun 01 '16 15:06 buu700

You have an editable area there, so you will definitely need an extended version :-)

Why not send code not formatted, and highilght upon rendering? That would reduce the amount of data to exchange..

asvd avatar Jun 01 '16 15:06 asvd

Yep, it sounds like the extended version would probably be relevant there.

That is what we're doing; what made you think otherwise? The text sent over the wire is just Markdown (foo.bar()).

buu700 avatar Jun 01 '16 15:06 buu700

Then it should work with the original version of microlight. You get the non-highlighted code, put it into a div with class=microlight, invoke microlight.reset() and have it highlighted. Right?

asvd avatar Jun 01 '16 16:06 asvd

Yeah, more or less; I would just have to change that line I linked to something like highlight: s => <div class='microlight'>${s}</div> and call microlight.reset() after it hits the DOM, as you noted.

The problem is that it would then have to bypass our post-processing logic (which currently does nothing that would impact code highlighting, but could eventually) and DOMPurify sanitisation, which would be particularly problematic for us in the event that someone were to find an XSS exploit against Microlight (probably unlikely given Microlight's limited scope, but all it would take would be for one browser to interact weirdly with Microlight in some edge case).

buu700 avatar Jun 01 '16 16:06 buu700

Just made the change you suggested earlier. Not that it really addresses your issue with accepting this PR, but the minified file size is back down to 2.2kb (gzipped is still 1.4kb).

buu700 avatar Jun 01 '16 17:06 buu700