Kdiff3 in kdereview

[Michael Reeves]

On Tue, Aug 28, 2018, 5:45 PM Albert Astals Cid <aacid at kde.org> wrote:

> El divendres, 24 d’agost de 2018, a les 3:20:13 CEST, Michael Reeves va
> escriure:
> > On Thu, Aug 23, 2018, 6:07 PM Albert Astals Cid <aacid at kde.org> wrote:
> >
> > > El dimarts, 7 d’agost de 2018, a les 14:59:50 CEST, Michael Reeves va
> > > escriure:
> > > > Kdiff3 has moved to review in preparation for possible release
> testing. I
> > > > am currently working towards having auto testing but the code needs
> major
> > > > refactoring to make this possible. Specifically it is not well
> > > modularized.
> > > > The purpose of this review is to get feedback on issues that need to
> be
> > > > addressed before moving out of playground.
> > >
> > > Have you seen there's 4 wrong connect signals on startup?
> > > https://paste.kde.org/pcvcje3u1
> > >
> > Not quite sure how to resolve this. How is scrolling content properly
> > implemented in qt5?
>
> I don't understand the question, what is missing is the signal you would
> emit from DiffTextWindow so it's DiffTextWindow saying it wants to scroll
> that is not something that it doesn't say anymore?
>

The code is a hack by the original author that tries to get notified when
the scroll bar moves. It happens to work as of qt5 but generates this
warning because QWidget::scroll is not a signal. Removing the offending
connects makes text in the diff mini window not scroll at all. How is
scrolling of content supposed to be implemented?

>
> > >
> > > When trying to compare any two files i hit this assert
> > >
> > >         if(m_lmppData.m_vSize < m_normalData.m_vSize)
> > >         {
> > >             //This a bug that needs fixed elsewhere not hacked around
> > >             Q_ASSERT(m_lmppData.m_vSize == m_normalData.m_vSize);
> > >
> > > Which i do not understand what it is trying to do, i mean you just
> checked
> > > that they are different and the on the next line assert they are not
> > > different?
> > >
> >
> > Actually that tells me what I need ed to know. I don't get this on my
> > machine. The comment made it seem like this was some sort of work around
> > for an odd corner case. I can remove the assert now that I know the
> trigger
> > is an everytime thing.
> > How are you doing the file comparison?
>
> kdiff3 file1 file2
>
> Cheers,
>   Albert
>
>
> > I feel like this points to an issue else where.
> >
> >
> > > Cheers,
> > >   Albert
> > >
> > >
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20180828/e535b2a2/attachment.htm>