<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 29, 2018, 4:34 PM Albert Astals Cid <<a href="mailto:aacid@kde.org">aacid@kde.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">El dimecres, 29 d’agost de 2018, a les 1:04:45 CEST, Michael Reeves va escriure:<br>
> On Tue, Aug 28, 2018, 5:45 PM Albert Astals Cid <<a href="mailto:aacid@kde.org" target="_blank" rel="noreferrer">aacid@kde.org</a>> wrote:<br>
> <br>
> > El divendres, 24 d’agost de 2018, a les 3:20:13 CEST, Michael Reeves va<br>
> > escriure:<br>
> > > On Thu, Aug 23, 2018, 6:07 PM Albert Astals Cid <<a href="mailto:aacid@kde.org" target="_blank" rel="noreferrer">aacid@kde.org</a>> wrote:<br>
> > ><br>
> > > > El dimarts, 7 d’agost de 2018, a les 14:59:50 CEST, Michael Reeves va<br>
> > > > escriure:<br>
> > > > > Kdiff3 has moved to review in preparation for possible release<br>
> > testing. I<br>
> > > > > am currently working towards having auto testing but the code needs<br>
> > major<br>
> > > > > refactoring to make this possible. Specifically it is not well<br>
> > > > modularized.<br>
> > > > > The purpose of this review is to get feedback on issues that need to<br>
> > be<br>
> > > > > addressed before moving out of playground.<br>
> > > ><br>
> > > > Have you seen there's 4 wrong connect signals on startup?<br>
> > > > <a href="https://paste.kde.org/pcvcje3u1" rel="noreferrer noreferrer" target="_blank">https://paste.kde.org/pcvcje3u1</a><br>
> > > ><br>
> > > Not quite sure how to resolve this. How is scrolling content properly<br>
> > > implemented in qt5?<br>
> ><br>
> > I don't understand the question, what is missing is the signal you would<br>
> > emit from DiffTextWindow so it's DiffTextWindow saying it wants to scroll<br>
> > that is not something that it doesn't say anymore?<br>
> ><br>
> <br>
> The code is a hack by the original author that tries to get notified when<br>
> the scroll bar moves. It happens to work as of qt5 but generates this<br>
> warning because QWidget::scroll is not a signal. Removing the offending<br>
> connects makes text in the diff mini window not scroll at all. How is<br>
> scrolling of content supposed to be implemented?<br>
<br>
Are you saying that removing a connect that is reporting to be broken changes the behaviour of the program?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I must be crazy anyway it seems to work now without those lines. </div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
That seems really strange, once you fix the assert if you tell me what to test i can have a look :)<br><br>
Cheers,<br>
  Albert<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div><div dir="auto">The assert will no longer happen that actually was committed right after you reported it. The code in question seems to be triggered when using preprocessing comands. I still need to look at this more closely but it should work as is. I have been reworking the file access code to try and simplify it. The diff process itself should not be affected buy this. This code base definitely feels something that was written under time constraints. It took a lot of effort to make it what is now. Not surprisingly there's still work to be done. One of my goals is to reduce make this code more easily maintainable. </div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> <br>
> ><br>
> > > ><br>
> > > > When trying to compare any two files i hit this assert<br>
> > > ><br>
> > > >         if(m_lmppData.m_vSize < m_normalData.m_vSize)<br>
> > > >         {<br>
> > > >             //This a bug that needs fixed elsewhere not hacked around<br>
> > > >             Q_ASSERT(m_lmppData.m_vSize == m_normalData.m_vSize);<br>
> > > ><br>
> > > > Which i do not understand what it is trying to do, i mean you just<br>
> > checked<br>
> > > > that they are different and the on the next line assert they are not<br>
> > > > different?<br>
> > > ><br>
> > ><br>
> > > Actually that tells me what I need ed to know. I don't get this on my<br>
> > > machine. The comment made it seem like this was some sort of work around<br>
> > > for an odd corner case. I can remove the assert now that I know the<br>
> > trigger<br>
> > > is an everytime thing.<br>
> > > How are you doing the file comparison?<br>
> ><br>
> > kdiff3 file1 file2<br>
> ><br>
> > Cheers,<br>
> >   Albert<br>
> ><br>
> ><br>
> > > I feel like this points to an issue else where.<br>
> > ><br>
> > ><br>
> > > > Cheers,<br>
> > > >   Albert<br>
> > > ><br>
> > > ><br>
> ><br>
> ><br>
> ><br>
> ><br>
> ><br>
<br>
<br>
<br>
<br>
</blockquote></div></div></div>