Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Unified versus Split Diff

matklad.github.io

Post reply on HN
Print view View original

111–120 of 184 posts

Re: Unified versus Split Diff

#111

Post by plugin-baby »

jaggederest wrote: post #71
crdrost wrote: post #29

Earlier quoted context omitted.

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, t…

In this kind of context, I ask people what log level they'd like their review at. If you just want to get the code out the door, by all means, "error" or "warn" might be the right review depth, when you're confident in your code and don't want to be derailed with philosophy. If you're exploring a new concept and want all the ideas and brainstorming you can get in your feedback, "debug" log level is appropriate. Once…

This is a really useful analogy, thank you for sharing it.

Re: Unified versus Split Diff

#112

Post by grotorea »

ilyt wrote: post #93
grotorea wrote: post #79

Earlier quoted context omitted.

I also wonder if it's possible to go beyond this project and have git itself work on the syntax level instead of pure text.

Git doesn't store diffs on logical level. Git operates on snapshots of trees. Commit is not "a collection of changes", it's a snapshot of a tree with attached predecessor of it. Then the another layer (which can be git, but also can be any other tool, adding custom diff tool to git is very easy) uses that to generate diffs. There is zero stopping anyone from adding contextual diffs to Git. Just ask it for content of…

Interesting. Wonder if something like that can be used to make git cleverer about merges for example.

Re: Unified versus Split Diff

#113

Post by sa46 »

larperdoodle wrote: post #86

I do all my large code reviews in IntelliJ. Seems like it's an exact match of what this guy wants? Diff view + full view + fully featured IDE for all his navigation, verification and search needs

What the author wants (and I do too) is a normal, full fledged editor that highlights the code under review. Bonus points for easy commenting.

You can sort of do it with IntelliJ with git blame annotations but it’s clunky. I don’t really care about the author’s commits; I care about the entire set of changes or the changes since my last review.

You can also sort of do it with IntelliJ’s pull request mode but it’s also clunky since it’s not a “real” editor and you lose highlighting if you jump to the source code.

Re: Unified versus Split Diff

#114

Post by gpvos »

The author complains that he doesn't see all of the code, but in the first image it looks like the right-hand pane shows exactly that (though with diff highlighting). Am I missing something?

Re: Unified versus Split Diff

#115

Post by gpvos »

e12e wrote: post #91

What is "a gpr script"? It looks like a small rust file that presumably compile to binary custom utility? https://github.com/matklad/config/blob/master/xtool/src/gpr....

It seems to be a script/program that just happens to be called gpr. I don't know the meaning of the abbreviation.

Re: Unified versus Split Diff

#116

Post by nh2 »

password4321 wrote: post #97

Semi-related, I recently discovered https://github.com/dandavison/delta : A syntax-highlighting pager for git, diff, and grep output It appears to enable choosing between unified and split views for each of those tools.

I'm currently waiting on the integration between Delta and Difftastic:

https://github.com/dandavison/delta/issues/535

Difftastic now has JSON output, whic should make it much easier to build this.

Re: Unified versus Split Diff

#117

Post by flanbiscuit »

phailhaus wrote: post #83

If you hit `.` in GitHub, you'll get dropped into a full IDE inside the browser. I've found this to be invaluable for reviews, because it lets you see the changes within the context of the entire file, rather than just seeing snippets. I'm much more likely to catch subtle design issues that way.

ok this is really cool, thanks for the tip! I've been in this IDE before but only when making edits to files directly in Github. I did not know I could shortcut to it directly from a commit diff.

By the way, this shortcut also works in Gitlab (just tried it)

Re: Unified versus Split Diff

#118

Post by codemac »

friendzis wrote: post #2

> 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…

Most of modern technology company culture has strayed too far from doing more formal processes for managing development.

By not having a basic process for defects, features, etc, every engineer has to make up and invent the process for how they will ship their feature through collaboration.

In most cases this ends up with doing little to nothing, with code review being the only form of collaboration on the topic that is in any meaningful detail. So we're left trying to stuff all possible interventions into a single moment.

Re: Unified versus Split Diff

#119

Post by wellpast »

> 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, but mostly just doing general review, as if I am writing the code. 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.

This is spot on. In fact, I think modern code review practices over emphasize the historical path to the code at the expensive of lost quality of the present code and code architecture.

"Minimizing diffs" is a feature of modern code and PR practices, whether it's explicit or tacitly something the developers do. Optimizing for minimizing diffs discourages the continuous refactoring that code bases require to stay solid, sound, and visibly correct.

Re: Unified versus Split Diff

#120

Post by IshKebab »

klauserc wrote: post #14
lelanthran wrote: post #7

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 author is the author of the Rust VSCode extension so I'm guessing he is familiar with VSCode.

If you use Gitlens's "compare working tree with..." you get the split view and you can edit the "current" version and all IDE tools work.

Post reply on HN