Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for loading all large diffs at once #825

Closed
monperrus opened this issue Nov 22, 2017 · 22 comments · Fixed by #2087
Closed

Add support for loading all large diffs at once #825

monperrus opened this issue Nov 22, 2017 · 22 comments · Fixed by #2087
Labels
enhancement help wanted small Issues that new contributors can pick up

Comments

@monperrus
Copy link

monperrus commented Nov 22, 2017

Issuehunt badges

by default larges diffs are not shown. If a PR contains many large diffs, one has to go over them one by one and there is a risk to miss something in a code review.

It would be great to have a button at the top "load all diffs".


IssueHunt Summary

notlmn notlmn has been rewarded.

Backers (Total: $40.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus sindresorhus changed the title add support for loading all large diffs at once Add support for loading all large diffs at once Nov 22, 2017
@sgabler
Copy link

sgabler commented Nov 22, 2017

Just curious: when do you need to check those large diffs? For me, those are usually auto-generated files that I can safely skip. What is your scenario?

@yakov116
Copy link
Member

yakov116 commented Nov 22, 2017

@sgabler a feature branch can sometimes have many files changed drastically

@monperrus
Copy link
Author

monperrus commented Nov 22, 2017 via email

@hkdobrev
Copy link
Contributor

I think GitHub had something like this the first time they've stopped rendering large diffs, but they've removed it. It was putting too much load for them to render them.

@fregante
Copy link
Member

fregante commented Feb 14, 2018

➡️ This should follow #821, #991 and #1042

@grgustaf
Copy link

Thumbs up on this... in particular it's frustrating that I do a painstaking code review, and the biggest file in the patch gets collapsed like this so that the author doesn't even notice all my comments in there as they leaf through. I'd suggest that diffs with comments be opened/loaded by default!

@fregante
Copy link
Member

No default, GitHub hid them because large diffs are heavy on their servers.

@vchong
Copy link

vchong commented May 16, 2018

+1 for a button at the top to "load all diffs". An additional possible use case is having to search for all occurrences of a particular string/function/etc in the diff, and hiding the large diffs hides some/all of these occurrences.

@leventov
Copy link

Anybody knows of any progress regarding this feature?

@fregante
Copy link
Member

fregante commented Nov 24, 2018

#1042 was dropped because GitHub implemented it natively.

If anyone wants to send a PR, it's probably a matter of:

  • restore+rename the feature
  • replace the selectors

@fregante fregante added help wanted small Issues that new contributors can pick up and removed under discussion labels Feb 13, 2019
@IssueHuntBot
Copy link

@IssueHunt has funded $40.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@fregante fregante added the has PR label Jun 5, 2019
@BillGeoghegan
Copy link

I used this snippet in my console to see how well it would perform.

document.querySelectorAll('[aria-label="Display the rich diff"]').forEach(button => {
  button.click();
});

The result... It performed sort of poorly with about ~100 backstop references in one PR loading their diff all at once, but it was certainly a lot faster than clicking all the diff buttons. 😄

@monperrus
Copy link
Author

Cool! Any way to embed this in a GreaseMonkey/TamperMonkey script?

@jm991
Copy link

jm991 commented Jul 12, 2019

FWIW I have this issue on my team almost daily (games team) and I wrote some hacky JS to click all the buttons. Just press Ctrl+Shift+I in Chrome (Inspect) and run this in the console:

 var buttons = document.getElementsByClassName('load-diff-button');

 for(var i = 0; i < buttons.length; i++)  
     buttons[i].click();

image

It'll take a bit, but it'll click all of them for ya. No more going crazy clicking buttons.

@monperrus
Copy link
Author

Excellent, thanks a lot! Will that be activated by default?

@fregante
Copy link
Member

fregante commented Sep 3, 2019

It's not a default but you can click on any "Load diff" while holding the alt key, and all the collapsed diffs will be loaded — as long as they appear in the dom at that time (huge PRs may be paginated so you might find more "Load diff"s later)

@issuehunt-oss
Copy link

issuehunt-oss bot commented Oct 31, 2019

@sindresorhus has rewarded $36.00 to @notlmn. See it on IssueHunt

  • 💰 Total deposit: $40.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $4.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Oct 31, 2019
@aryehbeitz
Copy link

aryehbeitz commented Mar 1, 2020

based on jm991's answer, I made it into a one liner:

Array.from(document.getElementsByClassName('load-diff-button')).map(button => button.click())

@jumasheff
Copy link

One more stuff: document.querySelectorAll('.load-diff-button').forEach(node => node.click()) Source

@jacobrask
Copy link

Try this if you have a large PR, it will schedule the clicks

document.querySelectorAll('[aria-label="Display the rich diff"]').forEach(button => {
  requestIdleCallback(() => button.click())
})

@cecton
Copy link

cecton commented May 23, 2022

It would be very nice to have a setting to enable this by default so the user don't have to press alt and click on one at all.

@cheap-glitch
Copy link
Member

@cecton Sorry, but we don't do per-feature settings (it's a recipe for scope creep)

@refined-github refined-github locked and limited conversation to collaborators May 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement help wanted small Issues that new contributors can pick up
Development

Successfully merging a pull request may close this issue.