Hacker News new | comments | show | ask | jobs | submit login

This ruins the readability and usability of your diffs. Say you need to quickly track down a major bug due to a change in a single constant. With horizontal alignment, the diff might contain any number of changed lines, obscuring the crucial change. There are workarounds that ignore whitespace and word-based diffs, but it's just not worth the trouble IMHO.




The problem I was alluding to occurs when a change causes the amount of alignment spaces to change, which then affects all the lines that have been aligned. Without alignment, the diff would be limited to just the code that was changed.


We should adjust our code and text to `diff`, rather than adjusting our tools to our code and text?


While I agree diff should be adjusted to accomodate for whitespace diffs more easily by default (it can do that with some options), it's not just a burden on the reviewer. It is also a burden on the programmer.

If you have, say, 50 lines of assignment and you align all the values to the largest one, adding one forces you to update 50 lines. I've been faced with those very situations and that is when I understood how important it is not to align values like that.


I don't why the committer can't leave a message like:

> Formatting change. Use `wdiff` to confirm that changes are just stylistic

The reviewer runs `wdiff` and confirms that the commit is just a formatting change. If the language is not layout-aware, then he will know that none of the changes are "semantic". Now he can look over the changed lines themselves (not necessarily with `diff`; just looking at the changed lines themselves) and see if the change is worth it/in line with the project.

PS: Maybe there should be a "column diff", something that checks that one file uses the same alignment as another file. I'm not able to show it here since HN will truncate spaces between words ( ;) ), but the point is to check if two files uses the same alignment, for example that in

> var v = 12

the next variable declaration, the numbers line up. I don't know if that is worth it, and the check would only be valid for some parts of the files.


Sure the reviewer could run `wdiff`, but it's an extra manual step due to a completely cosmetic and useless formatting style. If you're perusing hundreds of diffs, deciding when to applying whitespace significant vs insignificant diffs is a distraction, and it can't be done automatically.

You can fantasize about better diffs. Why isn't a diff applied directly to the abstract syntax trees of a language, for example? However, I think part of the robustness of version control systems comes from keeping things simple, namely line-based diffs, and with that comes a preference for keeping line-based diffs short.


Indeed, because adjusting version control system to parse ever language under the sun is not a good idea, and that would be required to properly identify strictly formatting-related changes.




Applications are open for YC Winter 2018

Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | DMCA | Apply to YC | Contact

Search: