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

141–150 of 184 posts

Re: Unified versus Split Diff

#141

Post by inferiorhuman »

jakub_g wrote: post #75
inferiorhuman wrote: post #69

Earlier quoted context omitted.

Disadvantages: * S-L-O-W * Glitchy Which is a travesty because a side by side diff view with an out-of-line list of PR comments could be incredibly useful if it didn't involve spinning up a whole VSCode instance in the browser. In fact it's so useful that GH used to have a split view available without forcing people into their buzzword AI ML crypto blockchain cloud enabled dev environment nonsense. Good idea, abysmal…

GH still has a split view available. Above the diff, you have a settings icon which shows dropdown where you can changed between unified and split diff.

Well that's certainly more intuitive than the old split / unified button. Unfortunately that basically revives the old split diff view and doesn't provide the useful out-of-band comments. Basically from a quick glance the buzzword view has a lot of good UI ideas that should have made it into the normal views as they have nothing to with actually running in a buzzword environment.

FWIW I'm benchmarking this against a PR where github is showing ~43,000 lines changed so things that are manageable in smaller changesets don't always scale.

Edit: I should also add that while the split view does exist in the non-buzzword environment, and there's even a (ugh) combo box to allow you to navigate to the inline comments, the split view actually hides the comments so they're completely inaccessible. Using the combo box does… absolutely nothing.

Decent ideas, atrocious execution as is tradition.

Re: Unified versus Split Diff

#142

Post by aulin »

junke wrote: post #49
pama wrote: post #23

Earlier quoted context omitted.

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

smerge-refine is one keybinding I always use to highlight changes in 3-way diffs between base/mine, base/theirs, mine/theirs. The only problem is knowing what mine and theirs mean sometimes.

Also magit-ediff ('e' in any merge conflict) is great and maybe a little more intuitive than smerge

Re: Unified versus Split Diff

#143

Post by PH95VuimJjqBqy »

mattarm wrote: post #110
dylukes wrote: post #3

Earlier quoted context omitted.

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

Those are good things to consider in review, but I maintain that the answer might be "no" to one or more of those questions and still be acceptable. I'm old enough to have worked in the pre-code-review era. Things were fine. People still learned from each other, software could still be great or terrible, etc. It wasn't appreciably worse or better than things are today. > An implicit question in several of the above i…

> Put another way, code review is also about a level of trust. Will the committer be around next week? Are they on the same team as me? If yes, give them some leeway to commit incremental work and make improvements later. Not all incremental work need occur pre-commit. Mention areas for improvement, sure, but don't go overboard as a gatekeeper.

I think this is very important, especially the part about incremental improvement. too many see development as laying concrete where it has to be perfect rather than as an ongoing process.

and personally the only thing I find PR's good for is ensuring jackasses aren't doing stupid shit. And by stupid shit here I mean things like using floats for currency (I caught that w/i the last year), things of that nature.

But my preference is to work with people I can trust and at that point I don't give a crap about a PR or a code review.

Re: Unified versus Split Diff

#144

Post by nuancebydefault »

I'm amazed that default diff tools that come with git CLI or even git UI clients are so freakin' the opposite of intelligent. Even github or Atlassian web interfaces. No matter unified or split.

Change some spacing and the whole line is marked. Move an if statement 10 lines up and you see complete blocks of changes. Not even speaking about changed order of function implementations.

This seems a not-so-hard problem solved already a-million-times and I always fall back to use a local UI and configure it to use a commercial differ/merger I once bought a license for. Can't do that for code change reviews in the browser though.

Re: Unified versus Split Diff

#145

Post by PH95VuimJjqBqy »

eru wrote: post #46
yarekt wrote: post #34

Earlier quoted context omitted.

This is called Continuous Integration, and its a shame that the term got nicked to now mean "that thing that builds our PRs somewhere". The idea was that everyone pushes to main all the time, which basically reduces integration time to 0, as everyone is doing it every couple of minutes on big teams. After a while you learn how to not step on people's toes (Introduce new classes incrementally, use docblocs documenting…

Everyone committing to the main branch all the time sounds like a nightmare to me, to be honest. I would probably seriously consider quitting if this was forced on me. (Or rather, I would probably just run on my private git copy, and only pull every once in a while, and ignore that the main branch always changes.) When / how do you do code review in your suggested workflow?

It's called trunk-based development.

https://www.atlassian.com/continuous-delivery/continuous-int...

Re: Unified versus Split Diff

#146

Post by Vogtinator »

nuancebydefault wrote: post #144

I'm amazed that default diff tools that come with git CLI or even git UI clients are so freakin' the opposite of intelligent. Even github or Atlassian web interfaces. No matter unified or split. Change some spacing and the whole line is marked. Move an if statement 10 lines up and you see complete blocks of changes. Not even speaking about changed order of function implementations. This seems a not-so-hard problem so…

You can usually hide/ignore whitespace changes which result in the kind of diff you want.

Re: Unified versus Split Diff

#147

Post by nuancebydefault »

Vogtinator wrote: post #146
nuancebydefault wrote: post #144

I'm amazed that default diff tools that come with git CLI or even git UI clients are so freakin' the opposite of intelligent. Even github or Atlassian web interfaces. No matter unified or split. Change some spacing and the whole line is marked. Move an if statement 10 lines up and you see complete blocks of changes. Not even speaking about changed order of function implementations. This seems a not-so-hard problem so…

You can usually hide/ignore whitespace changes which result in the kind of diff you want.

In practice, reviewers are "nitpicking" on white space as well. Before passing my code to a reviewer, it helps to just show the changes of just that.

Re: Unified versus Split Diff

#148

Post by pierrebai »

"everyone seems to be happy reviewing diffs, rather than the actual code?"

The split view does show the actual new code, just with extra empty lines... so sure it is a bit distracting if a lot of lines have been modified, but it rarely comes that far.

My beef is more with some team's habit of doing too much work in a single pull request. Large diff are just a symptoms of that. I find that it is much more likely to introduce bugs and hard-to-find changes in behaviour in large changes. If you later find that something broke, finding what caused it in a large PR can be a real time waster.

Re: Unified versus Split Diff

#149

Post by pierrebai »

matklad wrote: post #45
amelius wrote: post #41

How does this work for deleted lines, since these don't have a corresponding line in the "current" view.

Like this: https://user-images.githubusercontent.com/1711539/277602101-...

Your examples don't explain how both side can possibly be aligned when the differences are large. If they don't align, then the two view are independent and it's hard to see the correspondence. If they do align then... I'm not sure what is the difference between your suggestion and split diff, except th eleft side has less highlights.

So, as I understand it, you are advocating split diff with SUBTLER grey highlights in the current version of the file?

Re: Unified versus Split Diff

#150

Post by yencabulator »

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…

> Yes, git underneath stores data as diffs but they are only vaguely related to logical structure of commits

And that's why we call that lower level compression trick "delta", not "diff".

Post reply on HN