Thanks, Peter. There's still bugs here, and I've spent a lot of time reworking the algorithms to fix these. I'm not ready to ship any of that code yet, though. Needs further tweaking and testing. Having test cases like these really help with that, and I'll add the files to my regression tests.
Christian On Wed, Jul 5, 2017 at 9:20 AM, Peter Hodgson <[email protected]> wrote: > Hi Chris, > > I've now replicated this with a very simple example. > > The original file is attached as LICENSE. > > I added a char and committed the change reflected in rb40.patch and > created the review request (that file is created via the download patch > link) > > I then committed a change removing the line starting with the word > LIABILITY and updated the review request, the full diff of which is the > file rb40_v2.patch (again created via the download patch link). > > To define the issue again: > orig - v2 Shows the correct diff, showing both the char addition and line > removal > orig - v1 Shows the correct diff, showing just the line change > v1 - v2 Is nonsense, the diff claims "This file contains only > whitespace changes" and shows the last 5 lines of the rev1 file vs the last > four of the rev2 file. It does not highlight the missing lines. > > I understand this is an old thread, if I don't hear back in a couple of > days I'll raise it afresh. > > Many thanks, > Peter > > > On Wednesday, 5 July 2017 16:35:28 UTC+1, Peter Hodgson wrote: >> >> Hi Chris, >> >> I just came across this same bug in 2.5.12 exactly as described by Paul >> so I'm presuming it wasn't fixed. >> >> What files do you need to debug this? I'll see if I can reproduce this >> without distributing our actual codebase. >> >> Thanks, >> Peter >> >> On Tuesday, 30 June 2015 22:37:02 UTC+1, Christian Hammond wrote: >>> >>> Hi Paul, >>> >>> We recently fixed a bug like this, but perhaps there's another issue >>> somewhere. >>> >>> In order to diagnose this, I'll need to have original copies of the >>> affected files, as well as both diffs. I won't be able to diagnose without >>> those, unfortunately. >>> >>> Christian >>> >>> -- >>> Christian Hammond - [email protected] >>> Review Board - https://www.reviewboard.org >>> Beanbag, Inc. - https://www.beanbaginc.com >>> >>> -----Original Message----- >>> From: Paul Fee <[email protected]> >>> Reply: [email protected] <[email protected]>> >>> Date: June 30, 2015 at 4:02:10 AM >>> To: [email protected] <[email protected]>> >>> Subject: RB 2.0.17 - Interdiff fails to highlight deleted code >>> >>> > Hi all, >>> > >>> > I'm using ReviewBoard 2.0.17 and see unexpected behaviour in the diff >>> > viewer. >>> > >>> > Steps to reproduce: >>> > >>> > 1. Change a few files (I'm using SVN) >>> > 2. rbt post >>> > 3. Publish review >>> > 4. Delete a group of lines from one of the files already changed. >>> > 5. rbt post -r >>> > 6. Publish review >>> > 7. Review entire diff: http://reviewboard/r/9581/diff/2/ - Result: >>> PASS >>> > 8. Review first interdiff: http://reviewboard/r/9581/diff/1/ - >>> Result: PASS >>> > 9. Review second interdiff: http://reviewboard/r/9581/diff/1-2/ - >>> Result: >>> > FAIL >>> > >>> > In the second interdiff, RB states that the file contains only >>> whitespace >>> > changes, this is incorrect as lines have been deleted. >>> > >>> > Expanding the entire file, the contain on the left and right diff >>> panels is >>> > correct, I can see that the deleted lines have gone, however they're >>> not >>> > highlighted in red as expected. >>> > >>> > I don't think this is related to caching as the following steps had no >>> > effect, the second interdiff consistently shows the same result. >>> > >>> > * systemctl restart memcached >>> > * systemctl restart httpd >>> > * View second interdiff with different browsers (Firefox and >>> Chromium), >>> > both show same results, hence not a browser cache issue. >>> > >>> > I'm running ReviewBoard on CentOS7 using EPEL packages. >>> > >>> > Let me know if you need more information to help recreate or fix this >>> bug. >>> > >>> > Thanks, >>> > Paul >>> > >>> > -- >>> > 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 [email protected]. >>> > 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 [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- Christian Hammond President/CEO of Beanbag <https://www.beanbaginc.com/> Makers of Review Board <https://www.reviewboard.org/> -- 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
