Live data from Hacker News

Unified versus Split Diff

matklad.github.io

21–30 of 184 posts

Re: Unified versus Split Diff

#21
post #18

A third (fourth?) option worth mentioning here is difftastic[0], which uses "structural" diffing (as opposed to line diffing) for more granular diff highlighting. [0] https://github.com/Wilfred/difftastic

This is a great approach indeed, pity it's not integrated widely enough (think it only recently got a proper structured json output other tools could use)

Re: Unified versus Split Diff

#22

I’m surprised how we’re all still using line based diffs and even seemingly stupid ones at that. But the tales I hear from across the pond of paid git alternatives which diff based on understanding the programming language seem to be pretty bad as well. Though for other reasons?

It might be because of formatting, but that is exactly what I would have expected them to handle well.

Anyway, I have used a FOSS that does that, difftastic [1], and it does a pretty good job at language diff'ing without the annoyance of formatting as I hypothesised earlier.

[1]: https://github.com/Wilfred/difftastic

Re: Unified versus Split Diff

#23

I'm probably missing something here: author says that the split diff doesn't work for him, but doesn't say why. His ideal diff is pretty much the same as a split-diff but with redundant context removed (context is only on the left, not on both left and right). What utility does he get out of removing redundant context on the RHS of the pane?

The author writes > I need to run tests, use goto definition and other editor navigation features, apply local changes to check if some things could have been written differently, look at the wider context to notice things that should have been changed, and in general notice anything that might be not quite right with the codebase, irrespective of the historical path to the current state of the code. The editors/IDEs…

Maybe time to try Emacs? The diff support is great. And magit is native to Emacs.

Re: Unified versus Split Diff

#24
post #3

> For a large change, I don’t want to do a “diff review”, I want to do a proper code review of a codebase at a particular instant in time, paying specific attention to the recently changed areas, obviously every team and ticket is different, but IMO unless the person doing the review is some sort of principal engineer mostly responsible for the code at large, this does not align with I would personally consider a cod…

> In my book a general code review is simple sanity check by a second pair of eyes, which can result in suggestions to use different API or use an API slightly differently. This is an impoverished view of code review. Code review is a principal mechanism for reducing individual code ownership, for propagating conventions, and for skill transfer. A good code review starts with a good PR: one that outlines what its goa…

Well put. I'd like to add that individual code ownership must not necessary be lived as a loner sitting on that code. Being an owner means hatching, keeping clean and also knowing when getting help is beneficial. Accepting your bias and asking for a look from a different perspective is not getting rid of or dispersing ownership in my view, it is a central part of it.

Often this includes getting feedback if that change can make problems outside the feature implemented or in the future, like introducing dependencies that cost more in total than they help locally.

Re: Unified versus Split Diff

#25

I'm probably missing something here: author says that the split diff doesn't work for him, but doesn't say why. His ideal diff is pretty much the same as a split-diff but with redundant context removed (context is only on the left, not on both left and right). What utility does he get out of removing redundant context on the RHS of the pane?

The author writes > I need to run tests, use goto definition and other editor navigation features, apply local changes to check if some things could have been written differently, look at the wider context to notice things that should have been changed, and in general notice anything that might be not quite right with the codebase, irrespective of the historical path to the current state of the code. The editors/IDEs…

One area where vscode+gitlens plugin beats IntelliJ, the plugin adds in fully featured editable RHS to diff view

Re: Unified versus Split Diff

#26

I'm probably missing something here: author says that the split diff doesn't work for him, but doesn't say why. His ideal diff is pretty much the same as a split-diff but with redundant context removed (context is only on the left, not on both left and right). What utility does he get out of removing redundant context on the RHS of the pane?

The author writes > I need to run tests, use goto definition and other editor navigation features, apply local changes to check if some things could have been written differently, look at the wider context to notice things that should have been changed, and in general notice anything that might be not quite right with the codebase, irrespective of the historical path to the current state of the code. The editors/IDEs…

> The editors/IDEs I've used usually only offer rudimentary support in diffs (goto defn, but only in the same file; maybe auto-completion, but usually not for newly added items; no refactoring functionality)

Sure, but (to me it seems that) that doesn't necessitate the display change he is advocating for.

A diff program that has nice code navigation (and/or other IDE features) would be great, but what does that have to do with whether it is displaying the common split-diff, or his take on the split-diff.

Re: Unified versus Split Diff

#27

I'm probably missing something here: author says that the split diff doesn't work for him, but doesn't say why. His ideal diff is pretty much the same as a split-diff but with redundant context removed (context is only on the left, not on both left and right). What utility does he get out of removing redundant context on the RHS of the pane?

LHS shows current code, without any modifications. By showing full context in one view (with code browsing capabilities!) and changes in the other, the author is able to browse the code and see what changes, if any, are applied in a particular context. In my understanding the author wants to flip the diff around: instead of looking at changes themselves, the author wants to look at code and see if there are any assoc…

> LHS shows current code, without any modifications. By showing full context in one view (with code browsing capabilities!) and changes in the other, the author is able to browse the code and see what changes, if any, are applied in a particular context.

That's the bit I don't understand - how does the "changes in the other" help if displayed only as the unified snippet[1]? To my mind, the magic sauce bit is the full code navigation, and there's no reason that the RHS pane has to be limited to a unified diff when it can simply show the whole file with higlighted lines, the way split diff views do on the RHS.

[1] Perhaps (and I'm only guessing here), that the RHS must show the entire unified diff for the entire changeset, and not just the unified diff for the current file. To me, that makes the proposal an upgrade from "either show unified diff, or split-diff, file-by-file".

Re: Unified versus Split Diff

#28
This might be a useage thing but author seems to already point out that for a large code review, these two options aren't sufficient, but I've never seen these tools offer themselves as a solution to a large review.

They're a diff tool. The author's approach is creative but seems like it's the wrong tool for the job.

Re: Unified versus Split Diff

#29
post #3

> For a large change, I don’t want to do a “diff review”, I want to do a proper code review of a codebase at a particular instant in time, paying specific attention to the recently changed areas, obviously every team and ticket is different, but IMO unless the person doing the review is some sort of principal engineer mostly responsible for the code at large, this does not align with I would personally consider a cod…

> In my book a general code review is simple sanity check by a second pair of eyes, which can result in suggestions to use different API or use an API slightly differently. This is an impoverished view of code review. Code review is a principal mechanism for reducing individual code ownership, for propagating conventions, and for skill transfer. A good code review starts with a good PR: one that outlines what its goa…

I go back and forth on this.

On the one hand, I really like constant deep feedback. I really like the consistency benefits of having another person say “that’s too much, I find that unreadable.”

On the other, I have now been at a lot of places where it was very hard to get my code reviewed, latencies of days and sometimes weeks if folks are in a particularly heinous crunchtime... And then when it does get reviewed, the stuff that I worked really hard on to get the right speed or to properly centralize cache invalidation... Suddenly someone is like “I would have done it from this other approach” and you have no idea whether it's tractable or not.

While I have never been at a place that did this, I have in my head the idea that the code should be an unfolding collective conversation, kind of like when folks are all collaborating on a shared Google Doc, I see that you are editing this section and I throw in a quick comment “don't forget to add XYZ” and then jump to a different part that I won't be stepping on their toes with. So the basic idea would be to get everybody to merge to `main` like 2 or 3 times a day if possible. In that case code review really is just “make sure this doesn't break the build or break prod, everything is behind a feature toggle, if I don't like the design I will comment on the code near the design or sketch a proof of concept showing that my approach is superior in clarity or speed or whatever”... Nobody ever takes me up on this culture shift it seems.

Re: Unified versus Split Diff

#30

> For a large change, I don’t want to do a “diff review”, I want to do a proper code review of a codebase at a particular instant in time, paying specific attention to the recently changed areas, obviously every team and ticket is different, but IMO unless the person doing the review is some sort of principal engineer mostly responsible for the code at large, this does not align with I would personally consider a cod…

The problem is that this isn't a great way to catch areas of the codebase that should've been updated, but weren't.
Post reply on HN