El divendres, 31 d’agost de 2018, a les 14:48:53 CEST, Michael Reeves va escriure: > On Wed, Aug 29, 2018, 4:34 PM Albert Astals Cid <aa...@kde.org> wrote: > > > El dimecres, 29 d’agost de 2018, a les 1:04:45 CEST, Michael Reeves va > > escriure: > > > On Tue, Aug 28, 2018, 5:45 PM Albert Astals Cid <aa...@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 <aa...@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? > > > > Are you saying that removing a connect that is reporting to be broken > > changes the behaviour of the program? > > > > I must be crazy anyway it seems to work now without those lines.
Good :) One minor thing, there seems to be some small issue with memory leaks on optiondialog. If you compile with cmake -DECM_ENABLE_SANITIZERS="undefined;address and then just run kdiff3 and close it, i get these leaks reported at the end (amogsnst others that are noise) https://paste.kde.org/pp5k6jc6u Cheers, Albert > > > > > That seems really strange, once you fix the assert if you tell me what to > > test i can have a look :) > > > > Cheers, > > Albert > > > > 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. > > > > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >