3 ms·
AST-assisted diffing is something we've looked into for Review Board, and it's something we'll probably add at some point. There's certainly some benefit to doi
by chipx86 10y ago
AST-assisted diffing is something we've looked into for Review Board, and it's something we'll probably add at some point. There's certainly some benefit to doing it.
However, you don't need it for the case of "I've moved the class into a subclass which caused an indentation change." We handle that case by tracking indentation-only changes and by tracking moved code.
For instance, if you were to move a class or a method or some logic around within a file, we show that it was moved but not modified, saving you some review time. You can move an entire function into a class, with no modifications (or just a line changed here and there to reference `self` or `this`) and we intelligently show that. Saves a ton of time during code review.
If you nested a bunch of things inside a new conditional or subclass without otherwise changing the lines, we highlight just the indentation changes while making it clear the rest of the line is the same. Nest a 5 thousand line function inside of a new class, and you'll only have to worry about reviewing the class definition, not the indented 5 thousand lines.
You could get some benefits to this logic with AST-assisted diffs, to help you better know where a function/class/loop/etc. begins and ends (particularly with languages like Python), which can help you better represent some changes. You also get the benefit of knowing up-front about syntax errors. I've also given some thought to being able to "zoom out" of the line-by-line changes to a file and let you focus on the high-level structure of the classes/methods/docs of a file and how those have changed.
The downside to AST-assisted diffs is compatibility with language changes. It's important to have a sane fallback when the file isn't exactly as you expect it to be (such as if the language adds some new form of syntactic sugar but you're on an older version of the product doing the AST-assisted diffing, or you're parsing something that claims to be in a certain language but is using special syntax meant to be pre-processed during builds).