On Sep 12, 2010, at 1:04 PM, Kelan Champagne wrote:

> For really large commits, I would like a way to see just the list of changed 
> files without having to load/show the full diff.  I think this would be nice 
> for 2 reasons: (1) to avoid the overhead of having to parse/render the whole 
> diff until you really want to see it, and (2) to avoid having the commit 
> detail's scrollview contents be so long that it's temperamental to scroll 
> just through the list of changed files at the top.

I think this is a good idea.

> With that goal in mind, I took a look at code to see what I could figure out.
> 
> I think part (2) could probably be solved by simply having a separate link to 
> show just the file list (when the "This is a large commit" is shown), with 
> some corresponding arguments to showDiff() (in history.js).
> 
> However, based on what I could figure out, solving part (1) is a bit trickier 
> because I don't think there is a way to get the list of changed files from 
> the 'git show --pretty=raw' output without parsing the entire output (though, 
> I'd love to be corrected if I'm wrong).
> 
> So, what I did was add a new NSTask in PBWebHistoryController.m to run 'git 
> diff --numstat --summary [REV]^!', and then have it call a corresponding 
> javascript function to build the filelist from that info.  I both need 
> '--numstat' to get the number of lines changed, and '--summary' to get the 
> creation/deletion status (so I can show the correct icons).  The advantage of 
> this approach is that the javascript can show the filelist without parsing 
> the full 'git show' output.  However, the downside is that it now has to run 
> 2 tasks.  So, I'm curious to know if people think this is a horrible 
> approach, or if the tradeoff is worth it.

I would probably do 'git show --numstat -M --summary <SHA>'. Commits with 
multiple parents will be reported properly. The -M will indicate files that are 
renamed.

> To give a few more details, I serialized these tasks, first calling 'git diff 
> --numstat' (because that should be shorter/quicker), and then in the callback 
> when that finishes it passes the result to the javascript and then kicks off 
> the NSTask to run the 'git show' command.  The downside of this approach is 
> that it always has to run both tasks, but it seems like the most 
> straight-forward way to go about it.

This should be fine. Not having to wait for large commits to parse should be a 
pretty good win.

> One additional perk is that we can now show the number of lines modified in a 
> file in the commit detail view (because that info is included in the 'diff 
> --numstat' output).
> 
>> 
> I'm pretty happy with the results so far, but plan to continue refining the 
> presentation of the filelist.  I will create a fork on github (probably based 
> on brotherbard's experimental branch, which is the version I use day-to-day) 
> to share the changes.  Also, is it still recommended to send the patch around 
> on this list?.   But, I'm curious to hear if anybody has any feedback on my 
> general approach so far, and if this is a feature people think will be useful.

Well I've wanted both the file list on large commits and the stats on file 
changes, so... yes!

After you finish send me a pull request on Github. I've received a few already 
and it's very helpful since there are so many forks to keep track of. Also I 
can comment on the patch and give suggestions before applying it.


> This was my first crack at the GitX source, but I use the app every day and 
> think it's great, especially for browsing the repo's history.  So, I am 
> excited to learn my way around the code and continue to help improve it!
> 
> Thanks,
> -Kelan
> 

--Nathan

http://brotherbard.com/





Reply via email to