diff-renderer icon indicating copy to clipboard operation
diff-renderer copied to clipboard

closes #11 and updated dependencies

Open johan-olsson opened this issue 8 years ago • 9 comments

I had to update some outdated dependencies too

johan-olsson avatar May 25 '17 00:05 johan-olsson

This is a complete rewrite. I don't even know how to review this. The idea of that minimalistic parser wasn't to support all html possibilities, but rather a small subset which is mostly needed for views.

In case of style tag, it opens a new language inside. I think you shouldn't even be using it in diff-renderer in order to keep its simplistic parser.

Second point was speed, it was that simple without any functions, just one loop for the performane reasons.

You even left a console statement inside.

I think its better you use a fork.

kof avatar May 25 '17 07:05 kof

If you want a full html-parser I would suggest to look at some high quality solutions, they will by slower by factor 20 but at least with less bugs. Parsing html with a regex is a bad idea.

kof avatar May 25 '17 07:05 kof

First of all I'm not done yet, second it's actually fast then your parser when dealing with non full page size html. Besides, if you actually read the code you would see that I don't use regex to parse html...

johan-olsson avatar May 25 '17 09:05 johan-olsson

Did you try to run the bench I have there? What is it saying?

kof avatar May 25 '17 09:05 kof

No I ran a jspref one

johan-olsson avatar May 25 '17 09:05 johan-olsson

So how does it look like, got a link?

kof avatar May 25 '17 09:05 kof

The test I have there is using the same lib as jsperf.com

kof avatar May 25 '17 09:05 kof

I have added you as a collaborator, I think I don't have enough time to dive deep into that pr. I wil simply let you work on it.

kof avatar May 25 '17 09:05 kof

Also you got access for publishing on npm as well

kof avatar May 25 '17 09:05 kof