sajingeo / rietveld

Automatically exported from code.google.com/p/rietveld
Apache License 2.0
0 stars 0 forks source link

[Feature request] Show full file context in diffs #129

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In the top right of the side-by-side diff view, there is a "context" dropdown, 
which lets you select 
up to 100 lines of context around the change.

It would be great if this extended to show *all* context.

I have hit this issue a couple times, where I wanted to read more context in 
the file I was reviewing 
and couldn't. Similarly, I wanted to comment on related line which needed to be 
changed, but 
couldn't since it wasn't within 100 lines of context.

Original issue reported on code.google.com by eroman@chromium.org on 2 Jul 2009 at 6:25

GoogleCodeExporter commented 9 years ago
You can fetch skipped lines by using the corresponding links within the code. 
But I 
think I understand your problem, this is only useful when you need to view a 
few more 
lines above or below the skipped lines. If you want to see a larger context 
than 100 
lines around each change, clicking "expand below/above" multiple times - 
especially on 
huge files - isn't really satisfying...

Original comment by albrecht.andi on 2 Jul 2009 at 6:38

GoogleCodeExporter commented 9 years ago
Oh I see.

Yeah, my use-case was I wanted to expand everything, in order to then be able 
to use the browser's "Find" 
command to search for some code I knew exists and needs updating.

And then double-click that found line to place a comment pointing out it needs 
to be updated as well.

Original comment by eroman@chromium.org on 2 Jul 2009 at 7:13

GoogleCodeExporter commented 9 years ago
I think it's fine to add some larger constants.  The suppression of unchanged 
blocks
was initially introduced because of the 1 MB response limit.  This has long been
increased to 10 MB.

Also, it might make sense to change the "10 lines more" links to "N lines more" 
where
N is the same as the context.

(BTW, I *always* click on the wrong link first.  I don't know why, but somehow 
my
brain thinks "above" means "below" and vice versa.  Maybe it's related to the 
fact
that "scrolling up" really moves the text *down* -- think about it...)

Original comment by gvanrossum@gmail.com on 3 Jul 2009 at 3:31

GoogleCodeExporter commented 9 years ago
Accepting this issue, the 100 lines context is outdated and I'll have a closer 
look 
how the expand links work (@Guido, I've the same "problem" with those links :)

Original comment by albrecht.andi on 3 Jul 2009 at 4:15

GoogleCodeExporter commented 9 years ago
Patch uploaded to codereview: http://codereview.appspot.com/91121

Original comment by albrecht.andi on 15 Jul 2009 at 4:51

GoogleCodeExporter commented 9 years ago
Committed r431 and live.

Original comment by albrecht.andi on 31 Jul 2009 at 6:53