Merge lp:~mwhudson/loggerhead/unified-by-default-sbs-by-ajax into lp:loggerhead

[Michael Hudson-Doyle]

[Michael Hudson-Doyle (mwhudson)]

Hi!

This branch changes the revision page to display unified diffs by default and can convert these to and from side-by-side diffs using javascript. It works in all the cases I've tested, but there may well be corner cases that I've missed -- please thrash it around.

It's not super fast, taking a second or so to convert a 500 line diff. I don't see any obvious ways to make it faster, so unless you can spot something dumb I propose not worrying too much about this.

Testing in IE would probably be a good idea :) It seems to work in Safari.

[Martin Albisetti (beuno)]

Hi Michael,
Thanks for keep on bringing awesomeness into loggerhead.

I have a few non-blocking comments and open questions:
* What about making the line number columns slightly tinted with red/green so you can link what they're related to better?
* I guess this means we're going back to unified as the default. Is that intentional?
* What do you think about moving the added/removed color explanations to the line below it, and maybe float:right?
* With big diffs, it makes the browser sweat quite a bit. Maybe we should not allow this madness with big-ish diffs?

review: Approve

[lp:~mwhudson/loggerhead/unified-by-default-sbs-by-ajax]

287. By Michael Hudson-Doyle on 2009-02-19

funky css to color line numbers too

[Michael Hudson-Doyle (mwhudson)]

> Thanks for keep on bringing awesomeness into loggerhead.

Glad you like it!

> I have a few non-blocking comments and open questions:
> * What about making the line number columns slightly tinted with red/green so you can link what
> they're related to better?

Good idea. r287 of the branch.

> * I guess this means we're going back to unified as the default. Is that intentional?

Semi... it's less processing client side to generate unified diffs. Maybe we should store the users last choice in a cookie and maybe change to sbs on domready?

> * What do you think about moving the added/removed color explanations to the line below it, and
> maybe float:right?

I don't understand this suggestion, I'm afraid.

> * With big diffs, it makes the browser sweat quite a bit. Maybe we should not allow this madness
> with big-ish diffs?

What's big-ish? I was vaguely thinking that a progress bar might be good, but that might be over complicated...

[Martin Albisetti (beuno)]

On Thu, Feb 19, 2009 at 7:02 PM, Michael Hudson
<email address hidden> wrote:
>> * I guess this means we're going back to unified as the default. Is that intentional?
>
> Semi... it's less processing client side to generate unified diffs. Maybe we should store the users last choice in a cookie and maybe change to sbs on domready?

Cookies would be ideal, yes :)

>> * What do you think about moving the added/removed color explanations to the line below it, and
>> maybe float:right?
>
> I don't understand this suggestion, I'm afraid.

Do you know those squares that let you know which color is what, to
the right of the link to swap unified/sbs? Those :)

>> * With big diffs, it makes the browser sweat quite a bit. Maybe we should not allow this madness
>> with big-ish diffs?
>
> What's big-ish? I was vaguely thinking that a progress bar might be good, but that might be over complicated...

I don't think we can have a progress bar, it's basically the browser
trying to do a lot and trying not to die.
big-ish is ~4k lines

--
Martin

[Michael Hudson-Doyle (mwhudson)]

I'm going to merge this now.

Martin Albisetti wrote:
> On Thu, Feb 19, 2009 at 7:02 PM, Michael Hudson
> <email address hidden> wrote:
>>> * I guess this means we're going back to unified as the default. Is that intentional?
>> Semi... it's less processing client side to generate unified diffs. Maybe we should store the users last choice in a cookie and maybe change to sbs on domready?
>
> Cookies would be ideal, yes :)

File a bug? :)

>
>>> * What do you think about moving the added/removed color explanations to the line below it, and
>>> maybe float:right?
>> I don't understand this suggestion, I'm afraid.
>
> Do you know those squares that let you know which color is what, to
> the right of the link to swap unified/sbs? Those :)

OK, I knew what you meant, but not what you meant to do with them..

>>> * With big diffs, it makes the browser sweat quite a bit. Maybe we should not allow this madness
>>> with big-ish diffs?
>> What's big-ish? I was vaguely thinking that a progress bar might be good, but that might be over complicated...
>
> I don't think we can have a progress bar, it's basically the browser
> trying to do a lot and trying not to die.
> big-ish is ~4k lines

File another bug? :)

Cheers,
mwh