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

51–60 of 184 posts

Re: Unified versus Split Diff

#51

Post by Karliss »

difosfor wrote: post #17

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?

The way I look at it, I doubt there will be widespread use of structural diffing while code editing itself is based on plain text files consisting of lines and characters.

Structured diffing requires structural understanding of the code. But only tool that is capable to properly understanding the code is compiler you are using to compiler, and even that only if you don't have mistakes. Version control and thus diffing needs to be robust. Knowing how many times I have seen intelisense (or similar IDE) fail due to various reasons, i wouldn't want similar when doing diff. You don't want it to fail just because you used a new language feature which isn't supported by diff tool yet. I have seen failures even in syntax highlighters which only need a very simplified understanding of code.

I could see moving away from line based diffs if we switched to structural code editing and source code was saved as some kind of AST instead of plaintext, and editors enforced that only valid source code(at least in structural sense) can be saved.

There can be middleground like what the difftastic does with fallback to text based diffing. But I consider that as nice to have but optional functionality. Seeing how many developers have strong attachment, to specific tools and their existing workflow, I am not surprised about lack of adoption. Developers might not even be against those specific improvements, but the adoption can be easily blocked by lack of integration, lack of some unrelated functionality in the software that does have the integration, setting up process feeling like more effort than benefits of slightly better diff. I have also heard plenty of times the argument of "If you need fancy tools for code to be readable/workable then the problem is in your code not the tools". While there is some grain of truth in it, that doesn't mean you can't use better tools while still writing code which could be understood without them.

Line based diff is in the good enough territory. There are cases where structural diff can do a lot more but with good fraction actual code edits the output of structural diff will be the same as line based diff+smarter diff viewers which highlight the parts of line that were modified (instead of whole line) and option to hide whitespace changes.

Re: Unified versus Split Diff

#53

Post by gizzlon »

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?

You can have branches, but the ""rule"" is that no branch should live for more than, say, a day.

This trades the integration complexity and problems for some new.. challenges :)

Re: Unified versus Split Diff

#54

Post by vifon »

> The only thing I don’t get is automatic synchronization between magit status buffer, and the file that’s currently open in the editor. That is, to view the current file and the diff on the side, I have to manually open the diff and scroll it to the point I am currently looking at.

Probably not a perfect solution but `scroll-all-mode` should be pretty close, at least within a single file at a time.

Re: Unified versus Split Diff

#55

Post by psd1 »

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…

I'm early in my emacs journey, but used VSCode for years. The Github extension is absolutely amazing for pull requests, since you have all your LSP and regex code navigation goodness. I only used it for a small percentage of reviews, but it made it vastly easier to grok the impact of every change.

Re: Unified versus Split Diff

#56

Post by koolba »

crdrost wrote: post #29
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…

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…

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

You just discovered pair programming.

It works astonishingly well. The second pair of eyes not only catches errors and envisions expanded use cases, it also prevents you from shirking off to HN. The biggest problem is you need two people, preferably sitting right next to each other with just one person “driving”.

Re: Unified versus Split Diff

#57

Post by lapcat »

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…

In my book, superficial code review produces only superficial results. This is how so many bugs get shipped despite the ubiquity of code review.

Call me crazy, but I've always approached code review from the standpoint of a tester: I build and run the code, see whether it does what it's supposed to do, look for possible weak points, and try to break it.

The diffs are the beginning rather than the end. They show you where to start poking around in the code. The proof is in the pudding, though.

You might respond, "That's what testers are for, not engineers." But who can test better than someone who understands the code and knows exactly where to look and what to look for?

I know this attitude puts me in the minority. The majority of engineers seem to have an inborn horror at the prospect of, gasp, manually running code. (They're also strangely averse to using debuggers.) They'll do anything in the world and write vast infrastructure to avoid it. Automate all the things! But in my view, automation is just more possibly buggy code. Who tests the tests? Quis custodiet ipsos custodes?

Re: Unified versus Split Diff

#58

Post by pmontra »

In my experience unified diff is good for small changes. Split diff like meld is good for many changes in a long file. Many diffs in a long long file, you should not have such a file.

For large PRs with many files the problem is not so big because they are the sum of many small changes, file by file. Maybe a team should aim at small PRs but sometimes having to change X into Y everywhere, with a large X or Y, is an inescapable fact of life.

Re: Unified versus Split Diff

#59

Post by pmontra »

gizzlon wrote: post #53
eru wrote: post #46

Earlier quoted context omitted.

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?

You can have branches, but the ""rule"" is that no branch should live for more than, say, a day. This trades the integration complexity and problems for some new.. challenges :)

Do people start working on something new past 3 PM or do they start discussing what to do the next day?

Re: Unified versus Split Diff

#60

Post by dijksterhuis »

bernds74 wrote: post #44
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…

"I would have done it from this other approach". I've seen that, and it's not good when you get the feeling of "code review is when someone who hasn't thought about your problem tells you how you should have solved it". People sometimes feel they have to add value as a reviewer, and casually discarding other people's work is the way to do it. Fortunately it's not something I have to deal with at my current job.

I’ve made comments that start like that and it’s usually down to

1) obvious code smell, here’s an example using your existing code refactored and the reasons why it is a better fit here

2) you’ve done something I didn’t think of and it’s clearly better than the way I was thinking of it. Here’s why it’s better. Kudos!

Helps that I’m lead/principal in a small biz with like 4 people max writing code. So I kind of know what everyone is working on / what they’re touching with their changes.

Mileage will definitely vary in bigger teams / businesses. 100% helps that my performance isn’t tied to number of PRs reviewed etc.

Post reply on HN