search

znerol / node-delta

Delta.js - A JavaScript diff and patch engine for DOM trees
http://znerol.github.com/node-delta/
MIT License
46 stars 11 forks source link

Changes for making it work under node.js 4.x #5

Closed blambeau closed 8 years ago

blambeau commented 8 years ago

Hi @znerol,

Here are a few changes I've made for the lib to compile and work under node.js 4.4.3.

Can I ask you about your current plan for the library? I'd like to:

  1. Fix tests failing under node.js 4
  2. Complete the support for json/jsonml documents

Can I ask you whether those changes have any chance to be merged? Can I also ask you for clarifying the licensing terms. Is it a MIT license currently?

Thanks in advance, Bernard

znerol commented 8 years ago

Thank you for taking the time to file a PR with your changes.

Regarding 1: I just opened #6. Regarding 2: Sounds interesting. Problem with plain JSON is that node order is not specified. I guess that JsonML fixes that? Also do you need just parsing/serializing support of JsonML or do you also like to have a native patch format?

Regarding the test fix: That looks like a stupid oversight. I took the liberty to cherry-pick that at once.

znerol commented 8 years ago

@blambeau I'm trying to set up automated tests now. The only combination where tests pass right now is node 0.10.x and xmlshim 0.0.x. As soon as I switch to a newer xmlshim (i.e. newer jsdom version), tests begin to break. Before upgrading the dependencies I will try to fix the tests, such that it is easier to tell whether things are really working.

znerol commented 8 years ago

So, got it working with xmlshim 0.1.x on node 0.10 and 0.12, see https://travis-ci.org/znerol/node-delta/builds/145737538 (this is branch feature/master/travis)

blambeau commented 8 years ago

Hi @znerol

Great to hear! Under node 4.4.3, I've got two tests failing indeed: some hash code computations and HTML roundtrip. I planned to create an issue about them, but I see that you are aware.

I've completed my jsonml adapters by now, including a patch format. Let try to find a stable state, I'll rebase everything and create another pull request.

Regarding JSON, I think that in general json objects do not define a tree. So the problem is rather difficult to define in the first place. JsonML is a simple javascript encoding of XML/HTML trees, so that's much easier.

znerol commented 8 years ago

I've tagged 0.0.4 with support for node 4, 5, 6 and iojs 2, 3. I've cherry-picked the relevant commits from this PR, therefore closing this now. Thanks for your contributions so far!