search

sbenhaim / protobrew

Meteor.js wiki engine
4 stars 4 forks source link

htmldiff: initial ugly integration of htmldiff.js library #114

Closed posborne closed 10 years ago

posborne commented 10 years ago

This appears to be a better written library and I think a more promising route moving forward. There are some parts of the resultant output that are not desirable right now, but I think some of that can be resolved by some cleanup and styling changes.

@funkyeah This package is available on npm, but I wasn't sure how to integrate it so I just added it directly (as it was a single file and already written in coffeescript). It isn't "perfect" but seems to work better -- we may need to customize. Feedback on the right way to integrate appreciated.

posborne commented 10 years ago

It works. Its gettin' merged. feedback still very welcome.

funkyeah commented 10 years ago

Checkout ( https://github.com/oortcloud/unofficial-meteor-faq#what-about-npm-node-modules) for a decent overview of NPM use in meteor

the TL;DR is basically that you can A) you can require/use NPM packages by creating a meteorite package ( https://atmosphere.meteor.com/wtf/package) or B) install a 3rd party package that allows the use of straight Npm.require (in http://meteorhacks.com/complete-npm-integration-for-meteor.html)

I am down for going whatever direction on the diffing, I went away from htmldiff mostly because its inactive and looked less polished than things relying on diff match patch. For some of the reasons outlined in that email I just sent you, the diff match patch route has its own challenges (not really meant to work on tree structured documents like html), so you have to workaround it, and I am not sure if its worth it.

Given that its inactive its probably legit to just add it to our project. Where it lives depends on where you think it should live. I imagine either where it is, in client/lib, or some other place where the lib files all go.

On Thu, Dec 26, 2013 at 4:09 PM, Paul Osborne notifications@github.comwrote:

This appears to be a better written library and I think a more promising route moving forward. There are some parts of the resultant output that are not desirable right now, but I think some of that can be resolved by some cleanup and styling changes.

@funkeyah This package is available on npm, but I wasn't sure how to integrate it so I just added it directly (as it was a single file and already written in coffeescript). It isn't "perfect" but seems to work better -- we may need to customize. Feedback on the right way to integrate

appreciated.

You can merge this Pull Request by running

git pull https://github.com/sbenhaim/protobrew htmldiff

Or view, comment on, or merge it at:

https://github.com/sbenhaim/protobrew/pull/114 Commit Summary

  • htmldiff: initial ugly integration of htmldiff.js library

File Changes

  • M client/views/history/history.coffeehttps://github.com/sbenhaim/protobrew/pull/114/files#diff-0(89)
  • M client/views/history/history.htmlhttps://github.com/sbenhaim/protobrew/pull/114/files#diff-1(4)
  • A client/views/history/htmldiff/LICENSEhttps://github.com/sbenhaim/protobrew/pull/114/files#diff-2(19)
  • A client/views/history/htmldiff/htmldiff.coffeehttps://github.com/sbenhaim/protobrew/pull/114/files#diff-3(293)

Patch Links: