3 ms·
This is something that most code review tools apart from GitHub get right. For example Critic, Review Board, Phabricator and Gerrit all implement side-by-side d
by jgraham 12y ago
This is something that most code review tools apart from GitHub get right. For example Critic, Review Board, Phabricator and Gerrit all implement side-by-side diffs with correct syntax highlighting on each side. I don't know why the GitHub experience is so far from the state of the art.
- eridius 12y agoI would imagine it's easier to do syntax highlighting on side-by-side, because you can just reuse your existing syntax highlighter. But when presenting the `-/+` style of diff, you need a custom syntax highlighter that not only can skip over the `-/+` tokens but can also handle having two separate-but-intertwined non-contiguous highlighting states (because the `-` lines will be using a different state than the `+` lines). And then what do you do if the state ends up different when you hit the context lines again? There's no obvious choice as to how to highlight those.
- jgraham 12y agoRight, so the question is "why not have side-by-side diffs". The fact that syntax highlighting would work is one obvious advantage. It also means that you can read the before/after code linearly without having to mentally apply the patch. It seems like an obviously better UI for understanding the effect of changes (although of course I don't have data to back that up).
- Peaker 12y agoBecause side-by-side is very redundant, wasting almost half the screen on the exact same information. You don't have to "mentally apply" a patch, just ignore some lines of a certain color.
- eridius 12y agoI would like to see GitHub offer the option of seeing side-by-side. It can be helpful if you have a mess of tiny -/+ hunks right next to each other. But combined is definitely the correct default.
- ricardobeat 12y agoIt's easy to process highlighting for the entire previous file plus the new file, and simply preserve it per line.
- eridius 12y agoWhat do you mean? First off, you're making the assumption that your highlighting library gives you results that can be trivially split per-line (e.g. no styles cross line boundaries, line boundaries are easy to find, etc). I haven't worked with web highlighting libraries so I don't know if that's true. But let's say it is. Now how do you handle the problem I called out, where the context lines are highlighted differently in both versions? You only show the context lines once in a combined diff.
- dllthomas 12y ago"Now how do you handle the problem I called out, where the context lines are highlighted differently in both versions?" I'm not convinced "pick one" is a horrible choice.