8 ms·

Expanding Context in Diffs

112 points by nickpresta 13y ago

paddy_m 13y ago
I was just looking for this over the weekend, and found it this morning. When I talk about code to collaborators in email or chat, I always try to link to the file on github rather than just giving a path. I much prefer to recieve feedback that way. Maybe i'm not good enough with my text editor, but it is just a lot faster to get to the file that way.
bencoder 13y ago
Been needing this badly for reviewing changes to legacy code which tends to require a much larger context for understanding of a diff. Hope it shows up in GH enterprise soon!
- bnferguson 13y ago
  AFAIK it should be in the next major release (330)!
bcl 13y ago
When working locally you can do something similar with -U<n> and I've also found using --patience can sometimes generate cleaner patches.
yuvadam 13y ago
I hoped they would fix line wrapping - which should be a relatively simply task - by now. This is a very needed feature for projects that work a lot with text files (such as markdown) that do not automatically break on set widths (e.g. [1]) I've already contacted Github support with this request months ago, but no one's stepped up with a fix. Maybe this can get escalated somehow? Heck, I'd even submit a pull request if it was possible for github/github. [1] - https://github.com/cryptoparty/handbook/pull/72/files https://github.com/cryptoparty/handbook/pull/72/files
- sdesol 13y ago
  I'm not sure I understand what you are referring to. Are you saying you want the long lines to be wrapped in the diff display? If so, I really can't see how this is an issue with GitHub since this is how the unified diff output is suppose to look like. Displaying it any differently would be misleading since it is a long line.
  - ash 13y ago
    I think he wanted to see something like this instead: http://f.cl.ly/items/0B2T3i1o1A3y062q0K1Z/Image%202013.12.02%2023%3A00%3A58.png http://f.cl.ly/items/0B2T3i1o1A3y062q0K1Z/Image%202013.12.02... I don't find it too misleading. But long line could be made to look even more different (from multiple short lines) by having distinct color, typography marks etc.
- ngoldbaum 13y ago
  Why not add line breaks using your text editor? That will also help your collaborators who prefer not to wrap long lines. I have a hook in my .emacs that turns on auto-fill-mode whenever I open a .md or .rst file. I'd assume there's something analogous for vim or whichever editor you normally work with.
  - sdesol 13y ago
    I see this becoming a trend now, where long lines are used to make it easier to do diffs. I'm on the fence about this since there are pros and cons.
  - gabemart 13y ago
    In text, hard line breaks can be content rather than presentation, so adding them to the raw file is not always desirable. It would be fantastic if the github web viewers offered this feature. Also, won't it mess up diffs for wrapped text? If you add a few words in the middle of a paragraph and then re-wrap the text, won't the contents of every single following line be altered and show up as a diff?
ataggart 13y ago
Side-by-side diffs would be superior.
- tibbe 13y ago
  Aren't they complimentary? You can have a side-by-side diff without the extra context this change introduces. That said, I also want side-by-side diffs. They're necessary for more complex diffs.
- stormbrew 13y ago
  Agree with the adjacent post that this is a complementary feature request and I also really want it. There is a browser plugin that can sort of get you there, but it's pretty brittle.
- sdesol 13y ago
  I'm not sure if their stance has changed and I can't seem to find the answer that was given, but it had to do with aesthetics/layout. Basically their web layout width was too restrictive for showing side by side diffs. They only really have two options: 1) Do what Bitbucket does and have the side by side diff be a pop up window. I personally find this to be jarring and unnatural. 2) Combine both elements. It's what I do for my product. To better understand this take a look at http://ny.testdrive.gitsense.com/index?#pid=22&cid=27&trail= http://ny.testdrive.gitsense.com/index?#pid=22&cid=27&trail= and switch to the side by side view. If you hover over the line number, you'll be able to see the diff line in unified mode below. With this setup they can provide side by side without having the user to constantly scroll left to right.
philfreo 13y ago
Nice! I've been wanting this feature for a long time: http://philfreo.com/blog/the-problem-with-pull-requests-code-reviews/ http://philfreo.com/blog/the-problem-with-pull-requests-code...
DanielRibeiro 13y ago
Cool! Now the only thing missing is to integrate this[1] to natively support syntax highlight on diffs ;) [1] https://github.com/danielribeiro/github-diff-highlight-extension https://github.com/danielribeiro/github-diff-highlight-exten...
obiterdictum 13y ago
Worth noting that BitBucket had this as long as I can remember.
crassus 13y ago
If any githubbers are reading, it would be great to be able to filter my organization's timeline events out of my personal timeline events. It's been bugging me for years. Thanks.
- stormbrew 13y ago
  Oh man yes please to this. The timeline is effectively useless right now.
jakub_g 13y ago
Since we're touching code reviews, I'll plug my userscript here. It's useful for me for code reviews of big commits: you can fold the files, mark them as reviewed etc. and all of this is persisted in the local storage. You can install it from [1] if you know userscripts or read [2] otherwise. [1] http://userscripts.org/scripts/show/153049 http://userscripts.org/scripts/show/153049 [2] https://github.com/jakub-g/gh-code-review-assistant https://github.com/jakub-g/gh-code-review-assistant
johnny_reilly 13y ago
I love these guys
staticshock 13y ago
phabricator has had this feature forever. Here's an example diff: https://secure.phabricator.com/rP1b026fa6291fd86ed6a2e598e6a013b5b6124532 https://secure.phabricator.com/rP1b026fa6291fd86ed6a2e598e6a...
shurcooL 13y ago
> and the feature is available anywhere we render diffs No, it's not available on gist.github.com. Sure, they may (or may not) port it over after some time, but I just don't get why they can't use the same code in the first place. Why is GitHub reinventing the wheel in for its very own sub-site? (Other than that, this is absolutely great!)
mduan 13y ago
Since we're on the subject of Github's diff viewer, thought I'd throw in a plug for a chrome extension I wrote recently to improve it: https://chrome.google.com/webstore/detail/github-enhancement-suite/ahamcncifjblaomhphpfpopppadboiin https://chrome.google.com/webstore/detail/github-enhancement... Has side-by-side + word wrap + context (somewhat nicer looking than Github's, although could be biased) Largely inspired by Phabricator's diff viewer.
alxndr 13y ago
Fingers crossed for "default to ignore whitespace in all diffs" to be the next feature they roll out...

112 points by nickpresta 13y ago

I was just looking for this over the weekend, and found it this morning. When I talk about code to collaborators in email or chat, I always try to link to the file on github rather than just giving a path. I much prefer to recieve feedback that way. Maybe i'm not good enough with my text editor, but it is just a lot faster to get to the file that way.

bencoder 13y ago

Been needing this badly for reviewing changes to legacy code which tends to require a much larger context for understanding of a diff. Hope it shows up in GH enterprise soon!

bnferguson 13y ago
AFAIK it should be in the next major release (330)!

bcl 13y ago

When working locally you can do something similar with -U<n> and I've also found using --patience can sometimes generate cleaner patches.

yuvadam 13y ago

I hoped they would fix line wrapping - which should be a relatively simply task - by now. This is a very needed feature for projects that work a lot with text files (such as markdown) that do not automatically break on set widths (e.g. [1]) I've already contacted Github support with this request months ago, but no one's stepped up with a fix. Maybe this can get escalated somehow? Heck, I'd even submit a pull request if it was possible for github/github. [1] - https://github.com/cryptoparty/handbook/pull/72/files https://github.com/cryptoparty/handbook/pull/72/files

sdesol 13y ago
I'm not sure I understand what you are referring to. Are you saying you want the long lines to be wrapped in the diff display? If so, I really can't see how this is an issue with GitHub since this is how the unified diff output is suppose to look like. Displaying it any differently would be misleading since it is a long line.
- ash 13y ago
  I think he wanted to see something like this instead: http://f.cl.ly/items/0B2T3i1o1A3y062q0K1Z/Image%202013.12.02%2023%3A00%3A58.png http://f.cl.ly/items/0B2T3i1o1A3y062q0K1Z/Image%202013.12.02... I don't find it too misleading. But long line could be made to look even more different (from multiple short lines) by having distinct color, typography marks etc.
ngoldbaum 13y ago
Why not add line breaks using your text editor? That will also help your collaborators who prefer not to wrap long lines. I have a hook in my .emacs that turns on auto-fill-mode whenever I open a .md or .rst file. I'd assume there's something analogous for vim or whichever editor you normally work with.
- sdesol 13y ago
  I see this becoming a trend now, where long lines are used to make it easier to do diffs. I'm on the fence about this since there are pros and cons.
- gabemart 13y ago
  In text, hard line breaks can be content rather than presentation, so adding them to the raw file is not always desirable. It would be fantastic if the github web viewers offered this feature. Also, won't it mess up diffs for wrapped text? If you add a few words in the middle of a paragraph and then re-wrap the text, won't the contents of every single following line be altered and show up as a diff?

ataggart 13y ago

Side-by-side diffs would be superior.

tibbe 13y ago
Aren't they complimentary? You can have a side-by-side diff without the extra context this change introduces. That said, I also want side-by-side diffs. They're necessary for more complex diffs.
stormbrew 13y ago
Agree with the adjacent post that this is a complementary feature request and I also really want it. There is a browser plugin that can sort of get you there, but it's pretty brittle.
sdesol 13y ago
I'm not sure if their stance has changed and I can't seem to find the answer that was given, but it had to do with aesthetics/layout. Basically their web layout width was too restrictive for showing side by side diffs. They only really have two options: 1) Do what Bitbucket does and have the side by side diff be a pop up window. I personally find this to be jarring and unnatural. 2) Combine both elements. It's what I do for my product. To better understand this take a look at http://ny.testdrive.gitsense.com/index?#pid=22&cid=27&trail= http://ny.testdrive.gitsense.com/index?#pid=22&cid=27&trail= and switch to the side by side view. If you hover over the line number, you'll be able to see the diff line in unified mode below. With this setup they can provide side by side without having the user to constantly scroll left to right.

philfreo 13y ago

Nice! I've been wanting this feature for a long time: http://philfreo.com/blog/the-problem-with-pull-requests-code-reviews/ http://philfreo.com/blog/the-problem-with-pull-requests-code...

DanielRibeiro 13y ago

Cool! Now the only thing missing is to integrate this[1] to natively support syntax highlight on diffs ;) [1] https://github.com/danielribeiro/github-diff-highlight-extension https://github.com/danielribeiro/github-diff-highlight-exten...

obiterdictum 13y ago

Worth noting that BitBucket had this as long as I can remember.

crassus 13y ago

If any githubbers are reading, it would be great to be able to filter my organization's timeline events out of my personal timeline events. It's been bugging me for years. Thanks.

stormbrew 13y ago
Oh man yes please to this. The timeline is effectively useless right now.

jakub_g 13y ago

Since we're touching code reviews, I'll plug my userscript here. It's useful for me for code reviews of big commits: you can fold the files, mark them as reviewed etc. and all of this is persisted in the local storage. You can install it from [1] if you know userscripts or read [2] otherwise. [1] http://userscripts.org/scripts/show/153049 http://userscripts.org/scripts/show/153049 [2] https://github.com/jakub-g/gh-code-review-assistant https://github.com/jakub-g/gh-code-review-assistant

johnny_reilly 13y ago

I love these guys

staticshock 13y ago

phabricator has had this feature forever. Here's an example diff: https://secure.phabricator.com/rP1b026fa6291fd86ed6a2e598e6a013b5b6124532 https://secure.phabricator.com/rP1b026fa6291fd86ed6a2e598e6a...

shurcooL 13y ago

> and the feature is available anywhere we render diffs No, it's not available on gist.github.com. Sure, they may (or may not) port it over after some time, but I just don't get why they can't use the same code in the first place. Why is GitHub reinventing the wheel in for its very own sub-site? (Other than that, this is absolutely great!)

mduan 13y ago

Since we're on the subject of Github's diff viewer, thought I'd throw in a plug for a chrome extension I wrote recently to improve it: https://chrome.google.com/webstore/detail/github-enhancement-suite/ahamcncifjblaomhphpfpopppadboiin https://chrome.google.com/webstore/detail/github-enhancement... Has side-by-side + word wrap + context (somewhat nicer looking than Github's, although could be biased) Largely inspired by Phabricator's diff viewer.

alxndr 13y ago

Fingers crossed for "default to ignore whitespace in all diffs" to be the next feature they roll out...