Node diff #25
Merged
Node diff #25
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Undoes hours of overthinking things.
Finally found the right data structure for collecting and returning the diff information.
Begin adding `#isSame()` methods to all Node classes which need it for Diff. These two were put in place to test out some prototype Diff code, but will work.
Wrote the main body of the diffing algorithm. There are many conditionals and it may be possible to simplify or refactor, but it provides the framework for building out a semantic diffing utility.
We cycle through the diff to see if any of the Changes stored were actually just moved. Will probably actually store these in a separate object, but for now we are not.
This is a big improvement, but I've only just realized that a part of my approach has been wrongheaded. I need a way to measure the distance between two nodes, and can probably discard these ideas about complexity and depth. A simple levenshtein distance taken on the raw strings may be enough, or I may have to come up with some measurement specific to an AST.
It was not well thought out, was over complicating things, and producing inaccurate results. Easier to just go ahead and toss them in as an insertion and deletion, and then match up matches later.
This should have been 3 separate commits but the earlier commits didn't go through for some reason. The isSame stuff is fairly minor, but the NodeDiff and SequenceMatcher changes bring major improvements to the results.
enebo
added a commit
that referenced
this pull request
Sep 4, 2013
Node diff. I will merge and tweak/edit as needed. Cool stuff.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is a lot here, so please ask about anything. @enebo please look especially at my tests. I feel like they are brittle and I could really use some pointers on how to improve them.
In a twist of irony, just adding
isSame()to all the nodetypes ended up being all I really needed to fix up rsense. Have not yet implementedequals()but that should be easy enough, since it is just adding a comparison of position info and then checkingisSame().