Re: [RB][1.7.26] DiffViewer problems

[Christian Hammond]

Hi Lukasz,

We intentionally don't show line endings. For most repositories, most
setups, line endings are not a stable concept. That is, the line ending you
check into a repository is not necessarily the line ending you get when it
comes back out. It often depends on the platform, the type of repository,
the tool used to check out, and the configuration. Sometimes you're not
even dealing with \n vs \r\n, but sometimes \r, and sometimes \r\r\n. It
gets crazy. As these differences can easily result in failures to patch, we
do a lot of work to normalize the line endings prior to patching/rendering
the diff, meaning the original line endings are gone.

If you're working to fix up line endings in a repository, there's frankly
not much that will be reviewable by users, as we're not talking about logic
or code changes. It's best to take care of that outside the code review
process.

Git, by the way, may normalize line endings at clone/pull time as well. It
depends, again, on configuration and platform. See
https://help.github.com/articles/dealing-with-line-endings/

I'm not sure what you mean about different heights between revisions. Can
you show me a screenshot? Also, what version of Review Board are you using?

You probably know this, but 1.7 is very old at this point. It's no longer
supported by us. 2.0 is currently the oldest supported version of Review
Board, which will change when 3.0 comes out in a few months.

2.0 and higher have an extension mechanism that companies can use to
customize behavior without making upgrades harder. I don't know what kind
of customizations you have, but it might be worth moving those into an
extension. That's something we can talk about if you can tell me what sort
of customizations you have.

Christian



-- 
Christian Hammond
President/CEO of Beanbag 
Makers of Review Board 

On Mon, Apr 3, 2017 at 3:27 AM, Lukasz  wrote:

> Hello all RB developers and users,
> We have some problems with Review Board diff viewer.
>
> First one is related with line-endings.
> We add lots of files with text lines endings Unix-styled (LF) in first
> revision and then convert them into CR-LF (Windows) without changing its
> contents in second revision.
> We get for example, four pages of "changed files" (This diff has been
> split across 4 pages), but this list is shrinking while loading page, so
> finally we get only content-changed files.
>
> Summarize - we can see only couple of correctly changed files and four
> pages of empty diff files in which only line-endings have changed.
> Is Review Board in version 1.7 able to display somehow differences in line
> endings?
>
> Git is recognizing such change.
>
> Second problem - sometimes diff blocks are not displayed correctly (they
> are on different heights between revisions), maybe was there any fix for it?
>
> Our RB instance is strongly customized, that's why we can't update to
> latest version.
>
> Thanks for your help,
> Best regards,
> Lukasz
>
> --
> Supercharge your Review Board with Power Pack:
> https://www.reviewboard.org/powerpack/
> Want us to host Review Board for you? Check out RBCommons:
> https://rbcommons.com/
> Happy user? Let us know! https://www.reviewboard.org/users/
> ---
> You received this message because you are subscribed to the Google Groups
> "reviewboard" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to reviewboard+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
Supercharge your Review Board with Power Pack: 
https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: 
https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
--- 
You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[RB][1.7.26] DiffViewer problems

[Lukasz]

Hello all RB developers and users,
We have some problems with Review Board diff viewer.

First one is related with line-endings. 
We add lots of files with text lines endings Unix-styled (LF) in first 
revision and then convert them into CR-LF (Windows) without changing its 
contents in second revision.
We get for example, four pages of "changed files" (This diff has been split 
across 4 pages), but this list is shrinking while loading page, so finally 
we get only content-changed files.

Summarize - we can see only couple of correctly changed files and four 
pages of empty diff files in which only line-endings have changed.
Is Review Board in version 1.7 able to display somehow differences in line 
endings?

Git is recognizing such change.

Second problem - sometimes diff blocks are not displayed correctly (they 
are on different heights between revisions), maybe was there any fix for it?

Our RB instance is strongly customized, that's why we can't update to 
latest version.

Thanks for your help,
Best regards,
Lukasz

-- 
Supercharge your Review Board with Power Pack: 
https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: 
https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
--- 
You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[Christian Hammond]

Hi,

Our diff support is not designed to be extracted from Review Board, and we 
don't support its use in that way. Can you give me some insight into what 
you're working on?

Christian 

--  
Christian Hammond - christ...@beanbaginc.com  
Review Board - https://www.reviewboard.org  
Beanbag, Inc. - https://www.beanbaginc.com

-Original Message-
From: n Tomar 
Reply: reviewboard@googlegroups.com >
Date: June 2, 2015 at 11:28:00 AM
To: reviewboard@googlegroups.com >
Cc: anir...@anirudhsanjeev.org >
Subject:  Re: Want to use ReviewBoard's side-by-side diffviewer. Need 
documentation/help

> Hi,
> I want to use the diff functionality of reviewboard as it efficiently
> shows the moved code also. But Iam not sure whether I can be used in my
> application as it appears from the above conversation that diff code was
> earlier dependent on other modules in the reviewboard.
> Please suggest if it is now possible to use the diff module separately or
> not.
>  
> Thanks!
>  
> --
> Supercharge your Review Board with Power Pack: 
> https://www.reviewboard.org/powerpack/
> Want us to host Review Board for you? Check out RBCommons: 
> https://rbcommons.com/
> Happy user? Let us know! https://www.reviewboard.org/users/
> ---
> You received this message because you are subscribed to the Google Groups 
> "reviewboard"
> group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to reviewboard+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>  

-- 
Supercharge your Review Board with Power Pack: 
https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: 
https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
--- 
You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[n Tomar]

Hi, 
I want  to use the  diff functionality of reviewboard as it efficiently 
shows the moved code also. But Iam not sure whether I can be used in my 
application as it appears from the above conversation that diff code was 
earlier dependent on other modules in the reviewboard.
Please suggest if it is now possible to use the diff module separately or 
not.

Thanks!

-- 
Supercharge your Review Board with Power Pack: 
https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: 
https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
--- 
You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Upgrading from 1.6RC1 to 1.6RC2: There are unapplied evolutions for diffviewer.

[Christian Hammond]

Yeah, that's just a third-party thing. At that point in the process, we've
done a database sync, and we found out that an evolution needed to be
applied. The "Evolutions were successful" below indicates that when we got
to that point in the process, it went fine.

Maybe I can get a patch in to suppress that output or at least modify
rb-site to tell you not to worry about that.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Fri, Jul 29, 2011 at 1:54 PM, Joe Lencioni wrote:

> Specifically "There are unapplied evolutions for diffviewer." It
> appeared in a red color which made me think it might be a problem.
>
> On Fri, Jul 29, 2011 at 3:51 PM, Christian Hammond 
> wrote:
> > Hi Joe,
> >
> > Nothing in there looks problematic to me. What concerned you? Just things
> > like "No fixtures found" and "There are unapplied evolutions?"
> >
> > Christian
> >
> > --
> > Christian Hammond - chip...@chipx86.com
> > Review Board - http://www.reviewboard.org
> > VMware, Inc. - http://www.vmware.com
> >
> >
> > On Fri, Jul 29, 2011 at 1:05 PM, Joe Lencioni 
> > wrote:
> >>
> >> I just upgraded from 1.6RC1 to 1.6RC2 and when I ran rb-site upgrade, I
> >> got the following output:
> >> Rebuilding directory structure
> >> Updating database. This may take a while.
> >> Creating tables ...
> >> Upgrading Review Board from 1.6 RC1 to 1.6 RC2
> >> There are unapplied evolutions for diffviewer.
> >> Installing custom SQL ...
> >> Installing indexes ...
> >> No fixtures found.
> >> Evolution successful.
> >> Resetting in-database caches.
> >> Upgrade complete.
> >>
> >> After restarting my web server, everything seemed to work so I'm not too
> >> worried about this, but thought I should pass it along in case this is a
> >> problem that needs to be addressed.
> >>
> >> --
> >> Want to help the Review Board project? Donate today at
> >> http://www.reviewboard.org/donate/
> >> Happy user? Let us know at http://www.reviewboard.org/users/
> >> -~--~~~~--~~--~--~---
> >> To unsubscribe from this group, send email to
> >> reviewboard+unsubscr...@googlegroups.com
> >> For more options, visit this group at
> >> http://groups.google.com/group/reviewboard?hl=en
> >
> > --
> > Want to help the Review Board project? Donate today at
> > http://www.reviewboard.org/donate/
> > Happy user? Let us know at http://www.reviewboard.org/users/
> > -~--~~~~--~~--~--~---
> > To unsubscribe from this group, send email to
> > reviewboard+unsubscr...@googlegroups.com
> > For more options, visit this group at
> > http://groups.google.com/group/reviewboard?hl=en
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~--~~~~--~~--~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
>

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: Upgrading from 1.6RC1 to 1.6RC2: There are unapplied evolutions for diffviewer.

[Joe Lencioni]

Specifically "There are unapplied evolutions for diffviewer." It
appeared in a red color which made me think it might be a problem.

On Fri, Jul 29, 2011 at 3:51 PM, Christian Hammond  wrote:
> Hi Joe,
>
> Nothing in there looks problematic to me. What concerned you? Just things
> like "No fixtures found" and "There are unapplied evolutions?"
>
> Christian
>
> --
> Christian Hammond - chip...@chipx86.com
> Review Board - http://www.reviewboard.org
> VMware, Inc. - http://www.vmware.com
>
>
> On Fri, Jul 29, 2011 at 1:05 PM, Joe Lencioni 
> wrote:
>>
>> I just upgraded from 1.6RC1 to 1.6RC2 and when I ran rb-site upgrade, I
>> got the following output:
>> Rebuilding directory structure
>> Updating database. This may take a while.
>> Creating tables ...
>> Upgrading Review Board from 1.6 RC1 to 1.6 RC2
>> There are unapplied evolutions for diffviewer.
>> Installing custom SQL ...
>> Installing indexes ...
>> No fixtures found.
>> Evolution successful.
>> Resetting in-database caches.
>> Upgrade complete.
>>
>> After restarting my web server, everything seemed to work so I'm not too
>> worried about this, but thought I should pass it along in case this is a
>> problem that needs to be addressed.
>>
>> --
>> Want to help the Review Board project? Donate today at
>> http://www.reviewboard.org/donate/
>> Happy user? Let us know at http://www.reviewboard.org/users/
>> -~--~~~~--~~--~--~---
>> To unsubscribe from this group, send email to
>> reviewboard+unsubscr...@googlegroups.com
>> For more options, visit this group at
>> http://groups.google.com/group/reviewboard?hl=en
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~--~~~~--~~--~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: Upgrading from 1.6RC1 to 1.6RC2: There are unapplied evolutions for diffviewer.

[Christian Hammond]

Hi Joe,

Nothing in there looks problematic to me. What concerned you? Just things
like "No fixtures found" and "There are unapplied evolutions?"

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Fri, Jul 29, 2011 at 1:05 PM, Joe Lencioni wrote:

> I just upgraded from 1.6RC1 to 1.6RC2 and when I ran rb-site upgrade, I got
> the following output:
>
> Rebuilding directory structure
> Updating database. This may take a while.
> Creating tables ...
> Upgrading Review Board from 1.6 RC1 to 1.6 RC2
> There are unapplied evolutions for diffviewer.
> Installing custom SQL ...
> Installing indexes ...
> No fixtures found.
> Evolution successful.
> Resetting in-database caches.
> Upgrade complete.
>
>
> After restarting my web server, everything seemed to work so I'm not too
> worried about this, but thought I should pass it along in case this is a
> problem that needs to be addressed.
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~--~~~~--~~--~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Upgrading from 1.6RC1 to 1.6RC2: There are unapplied evolutions for diffviewer.

[Joe Lencioni]

I just upgraded from 1.6RC1 to 1.6RC2 and when I ran rb-site upgrade, I got 
the following output:

Rebuilding directory structure
Updating database. This may take a while.
Creating tables ...
Upgrading Review Board from 1.6 RC1 to 1.6 RC2
There are unapplied evolutions for diffviewer.
Installing custom SQL ...
Installing indexes ...
No fixtures found.
Evolution successful.
Resetting in-database caches.
Upgrade complete.


After restarting my web server, everything seemed to work so I'm not too 
worried about this, but thought I should pass it along in case this is a 
problem that needs to be addressed.

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: No new line at end of file crash diffviewer

[Chris Clark]

Christian Hammond wrote:

What version of patch are you guys running?


I'm running Ubuntu 8.04.3 LTS - I have patch 2.5.9


   ing...@reviews:~$ cat /etc/*elease*
   DISTRIB_ID=Ubuntu
   DISTRIB_RELEASE=8.04
   DISTRIB_CODENAME=hardy
   DISTRIB_DESCRIPTION="Ubuntu 8.04.3 LTS"
   ing...@reviews:~$ patch -v
   patch 2.5.9
   Copyright (C) 1988 Larry Wall
   Copyright (C) 2003 Free Software Foundation, Inc.

   This program comes with NO WARRANTY, to the extent permitted by law.
   You may redistribute copies of this program
   under the terms of the GNU General Public License.
   For more information about these matters, see the file named COPYING.

   written by Larry Wall and Paul Eggert




And do the diffs have any indication at the end saying that there's no 
newline at the end?


Yes and no ;-) For me it depends on the diff tool used. E.g. some 
diff tools dump:


   \ No newline at end of file

at the point the newline is missing to stdout, for a single file diff 
this is at the end. For multiple file diffs this could be in the middle 
of the diff.


Some diff tools (e.g. Solaris) dump that information to stderr, if you 
redirect both stdout and stderr the position of that message can vary 
wildly.


Some diff tools do nothing. Which means that multi-file diffs with the 
"bad file" in the middle look corrupted as the next file starts on the 
last line of the previous files diff.





Also, what SCM?


I'm using p2 which has the option to use the platform provided diff (so 
sort of a lottery on the newline detection) OR perform diffs server side 
(which gives consistent output). The default diff format that is used 
for p2 is the default diff output (i.e. not a unified diff).


I'm not sure the SCM relevant, I think the issue is the diff and patch 
tools.


Chris

--
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: No new line at end of file crash diffviewer

[Christian Hammond]

What version of patch are you guys running? And do the diffs have any
indication at the end saying that there's no newline at the end?

Also, what SCM?

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Wed, Apr 28, 2010 at 1:41 PM, Chris Clark  wrote:

> Jan Koprowski wrote:
>
>> When I make review for file where some changes was made near last
>> line and it have not new line at end of file diffviewer crush :/ I'm
>> thinking about adding one more "fake" new line sign at the end
>> everytime. Is this patch will be submitted if I create one?
>>
>>
>
> That is probably not going to work.
>
> We've seen a few of these too :-(
>
> It isn't reviewboard that is complaining, it is the patch tool.
>
> I've been suggesting to my users that they make a separate submission into
> SCM that simply adds a newline to the end of file of any files that have
> this problem without going through reviewboard. Then (re-)edit with the real
> change and the diff will be fine as you would expect.
>
> Chris
>
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~--~~~~--~~--~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
>

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: No new line at end of file crash diffviewer

[Chris Clark]

Jan Koprowski wrote:

When I make review for file where some changes was made near last
line and it have not new line at end of file diffviewer crush :/ I'm
thinking about adding one more "fake" new line sign at the end
everytime. Is this patch will be submitted if I create one?
  


That is probably not going to work.

We've seen a few of these too :-(

It isn't reviewboard that is complaining, it is the patch tool.

I've been suggesting to my users that they make a separate submission 
into SCM that simply adds a newline to the end of file of any files that 
have this problem without going through reviewboard. Then (re-)edit with 
the real change and the diff will be fine as you would expect.


Chris

--
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

No new line at end of file crash diffviewer

[Jan Koprowski]

Hi!

  When I make review for file where some changes was made near last
line and it have not new line at end of file diffviewer crush :/ I'm
thinking about adding one more "fake" new line sign at the end
everytime. Is this patch will be submitted if I create one?

Greetings from Poland!
--
Jan Koprowski

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: diffviewer

[Christian Hammond]

Hi Damien,

The diff viewer was moved into its own app very early on for the purpose of
organization, but there aren't any plans to actually split it off into its
own third-party module short-term. It would increase our maintenance burden
a bit, and it depends too much on other modules within Review Board (such as
the scmtools and accounts modules).

That being said, with some changes to make the diffviewer's dependencies on
the accounts module a bit more flexible, you could potentially just depend
on the reviewboard codebase and use the diffviewer module directly from
that. It does, however, mean pulling in more code than you might prefer.

If you did want to make use of it, it would require some code changes. You'd
need to look at every import in scmtools/*.py and diffviewer/*.py and see
what other parts of reviewboard is required. You'd then need to work on
factoring those out in some way. If it's clean and doesn't increase our
maintenance burden, we'll take a patch for it. You'd need to then include
diffviewer and scmtools in your INSTALLED_APPS in settings.py.

There may be more, but not having had to do this, I'm not sure off-hand.

I'm not completely opposed to having this module as its own third-party
module, but it does make life a bit harder for us and our users, as we now
would have to match up versions and test various combinations.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Thu, Jan 28, 2010 at 2:47 PM, damdoum  wrote:

> Hello,
>
> I have questions about the diffviewer feature of reviewboard.
> Is it going to be forked off reviewboard and become a separate MIT-
> licensed django app? I could see that the code was moved around a bit
> to make it a separate app, but it that the direction it is taking?
> My second question would be how easy would it be to use that
> diffviewer feature on its own?
> - changing the settings.py?
> - a view that accepts post data / MIME multi-part?
> - use of URI to point to source / diff?
>
> thank you!
>
> Damien.
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~--~~~~--~~--~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

diffviewer

[damdoum]

Hello,

I have questions about the diffviewer feature of reviewboard.
Is it going to be forked off reviewboard and become a separate MIT-
licensed django app? I could see that the code was moved around a bit
to make it a separate app, but it that the direction it is taking?
My second question would be how easy would it be to use that
diffviewer feature on its own?
- changing the settings.py?
- a view that accepts post data / MIME multi-part?
- use of URI to point to source / diff?

thank you!

Damien.

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Re: Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[Anirudh Sanjeev]

Hi,

Thank you for the detailed and informative reply.

I had a closer look at the Review Board diff algorithm and was rather
overwhelmed. Re-using the code for my simple purpose would be overkill.
But there still isn't any tool to work on the unified diff format in python.
I found some respite with Google's "diff-match-patch" library, which I'm
currently playing around with. The library cannot handle the full blown
unified diff format (with the Index, ---, +++ lines, etc) but seems to
handle the diff chunks along with the range information quite well.

I proceeded to split the entire diff into chunks and passed it to the
patch library which converts it into a saner format, and I'm taking the 
output of that and converting it to a two-column HTML table, with the entry
appearing:
* on both sides if there isn't a change in the line
* on the left side if the line has been removed
* on the right if the line has been added.

I was hoping if you could tell me if I'm going in the right direction.

So far, I've been testing it on Review Board's test diffs in the
testdata/diffs/unified directory, and results have been acceptable. I'm
just concerned that the format might vary as I haven't seen many
test cases yet.

Thanks,
Anirudh

--
Senior Undergraduate student, Indian Institute of Technology, Kharagpur
http://anirudhsanjeev.org


signature.asc
Description: PGP signature

Re: Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[Christian Hammond]

Hi,

Yeah, the diff viewer in Review Board is certainly larger than other differs
such as difflib, but that's because it isn't just another difflib. It has:

* Database storage of the diffs it operates on
* A complete implementation of the Myers diff algorithm + optimizations.
Python's difflib uses a much simpler, inferior algorithm that doesn't even
have a concept of "replace" lines.
* Full side-by-side diffs that can show either the entire file with the diff
applied or just the changed segments.
* Syntax highlighting
* Interdiffs (diffs between revisions of a diff)
* Move detection (showing whether or not a range of lines have moved from
one location in a file to another)
* Inter-line diffs (What parts of a line changed in a "replace" line)
* The ability to "watch" for certain lines in one of the files in the diff
and mark the location and string for later processing (such as finding where
classes/functions are and representing them).
* Intelligent whitespace handling and markup, so that you know whether a
particular line only contains whitespace and can thus be hidden if needed.

Python's difflib is something that you would use for generating a unified
diff, not something that parses an existing one. Truth be told, you don't
want to go the route of writing code that applies a diff to a buffer/file.
You want to use patch for this. The reason is that, while they seem simple,
diffs come in all shapes and sizes. I'm not talking unified vs. context, but
rather subtleties in how various implementations even generate a unified
diff, what they include in there, etc. If you ever look at the source for
GNU patch, you'll run away.

The method Review Board uses is to take the unified diff, parse it just
enough to split it into one diff per file, and then actually use GNU patch.
We fetch a file from the repository, save it in a buffer as the original
file (left-hand side of the diff), and to a temp file, patch the temp file
with the diff, and then read that back in as the modified file (right-hand
side of the diff). You can then use either the Myers differ in Review Board
(recommended -- and standalone) or Python's difflib (won't give you the
results you'd normally expect from a differ in many cases).

Now, what I've said has assumed that you're going to have the original file
to display, instead of just the diff. If it's just the diff, then the Review
Board diff viewer won't help you. Nor will Python's difflib. What you'll
need to do is custom-parse the diff. You'll have to handle the headers
indicating the files, and the lines. For the lines, you'll need to keep
records on each line and whether it's unmodified, insert, delete, or
replace. For unmodified, insert and delete, you can check if the line starts
with " ", "+" or "-". For replace, you're sort of out of luck. You can try
some heuristics, but it'll get it wrong part of the time. Review Board used
to do this before our Myers differ (as Python's difflib doesn't generate
"replace" information), and it was just bad.

If you do go the route of parsing the diffs and need to parse it into files,
you can look at reviewboard/diffviewer/parser.py and the various DiffParser
subclasses in reviewboard/scmtools/*.py. It won't help you get the
insert/delete/remove lines, though, as that's something we get by patching
the file and then running the Myers differ across both.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Sat, Jan 9, 2010 at 2:06 PM, Anirudh Sanjeev
wrote:

> Hi!
>
> Excerpts from Christian Hammond's message of Sun Jan 10 03:18:17 +0530
> 2010:
> > There are no plans to take the diff viewer and fully separate it out as
> an
> > independent, reusable component, as it's a lot of work and increases our
> > maintenance burden.
>
> I see your rationale behind this. However my needs are quite simple - I
> have a patch in the unified diff format which needs to be converted into
> the side-by-side HTML view which shouldn't be too hard to implement from
> scratch.
>
> I was hoping someone could merely point me at where to look at the
> relatively large ReviewBoard codebase which I am using only as
> reference.
>
> I looked in the diffviewer app, but that seems to be rather large and
> daunting. Most other implementations seem to be quite inferior to
> ReviewBoard's implementation of the same. Python's difflib's HTML table
> generator is enough for my needs, but unfortunately, I couldn't find a
> good enough way to convert unified diff to the difflib object structure.
>
> Again, any pointers on where to start looking at 

Re: Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[Anirudh Sanjeev]

Hi!

Excerpts from Christian Hammond's message of Sun Jan 10 03:18:17 +0530 2010:
> There are no plans to take the diff viewer and fully separate it out as an
> independent, reusable component, as it's a lot of work and increases our
> maintenance burden.

I see your rationale behind this. However my needs are quite simple - I
have a patch in the unified diff format which needs to be converted into
the side-by-side HTML view which shouldn't be too hard to implement from
scratch.

I was hoping someone could merely point me at where to look at the
relatively large ReviewBoard codebase which I am using only as
reference.

I looked in the diffviewer app, but that seems to be rather large and
daunting. Most other implementations seem to be quite inferior to
ReviewBoard's implementation of the same. Python's difflib's HTML table
generator is enough for my needs, but unfortunately, I couldn't find a
good enough way to convert unified diff to the difflib object structure.

Again, any pointers on where to start looking at the code would be
awesome!

Thank you,
Anirudh

--
Senior Undergraduate student, Indian Institute of Technology, Kharagpur
http://anirudhsanjeev.org


signature.asc
Description: PGP signature

Re: Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[Christian Hammond]

Hi,

Review Board's diff viewer is modular (well, in theory, but we've only used
it in Review Board). It should be usable in another project, though it
requires other pieces of the Review Board codebase (like the SCMTools, admin
settings, and accounts code). If you required Review Board as a dependency,
and based some of your logic and profiles on ours, then you might be able to
use it.

There are no plans to take the diff viewer and fully separate it out as an
independent, reusable component, as it's a lot of work and increases our
maintenance burden. However, I would take patches that reduce its
requirement on other parts of our codebase, if they make sense and don't
complicate things too much.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Sat, Jan 9, 2010 at 9:52 AM, Anirudh  wrote:

> Hi,
>
> I'm writing an open-source project in django. It's like pastebin for
> patches where each patch gets it's own review thread. Think
> reviewboard, but where each patch gets it's own url for review, and it
> is project-agnostic. A user can upload/paste a patch and get a link.
> This is primarily being done as a learning exercise.
>
> I looked at different diff/patch libraries to parse and view the
> patches. None seem as good as the Reviewboard's unified diff parsing
> system. My current requirement is only something that parses unified
> diffs provided as text and outputs HTML for the side-by-side diffs.
>
> Any help/feedback/suggestions will be greatly appreciated.
>
> Thank you,
> Anirudh
>
> --
> Senior Undergraduate student, Indian Institute of Technology,
> Kharagpur
> http://anirudhsanjeev.org
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~--~~~~--~~--~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
>
-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Want to use ReviewBoard's side-by-side diffviewer. Need documentation/help

[Anirudh]

Hi,

I'm writing an open-source project in django. It's like pastebin for
patches where each patch gets it's own review thread. Think
reviewboard, but where each patch gets it's own url for review, and it
is project-agnostic. A user can upload/paste a patch and get a link.
This is primarily being done as a learning exercise.

I looked at different diff/patch libraries to parse and view the
patches. None seem as good as the Reviewboard's unified diff parsing
system. My current requirement is only something that parses unified
diffs provided as text and outputs HTML for the side-by-side diffs.

Any help/feedback/suggestions will be greatly appreciated.

Thank you,
Anirudh

--
Senior Undergraduate student, Indian Institute of Technology,
Kharagpur
http://anirudhsanjeev.org
-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

cache_memoize large objects in diffviewer/views.py

[plumpy]

While tracking down some caching issues we're having, I noticed that
build_diff_fragment in diffviewer/views.py isn't using the large_data
option to cache_memoize. Should it be? (That's not a rhetorical
question.)

I doubt this is the source of the problems we're having, but I noticed
it and thought I'd ask.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To post to this group, send email to reviewboard@googlegroups.com
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~--~~~~--~~--~--~---