Trying to port from the deprecated KDialog to make KFind pure KF5 application.
Details
Details
- Reviewers
broulik - Group Reviewers
KDE Applications - Commits
- R228:c07b7d7066a2: Port KFind away from KDialog
- Should be compilable without warnings on KDialog deprecation.
- Visually indistinguishable from the unported version (resizing of the window works just the same even without hints).
- Crashes after pushing the "Search" button. For some reason, it is impossible for my code to change the enable state of the buttons.
Can somebody give a hint on where did I go wrong? Thanks in advance.
Diff Detail
Diff Detail
- Repository
- R228 KFind
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
The order of the top right buttons is also reversed now, "Search" is the bottom-most button in the section now.
| src/kfinddlg.cpp | ||
|---|---|---|
| 32 | Put Qt includes with the other ones above and sort alphabetically (only the ones you add, don't change the surrounding code :) | |
| 75 | Coding style: put spaces around | | |
| 76 | The crash is because you create a local variable and as such the member variable in the class is never assigned and calling it in e.g. startSearch crashes as it accesses uninitialized memory. That's why we typically prefix member variables with m_, which isn't the case in this old legacy code. user1Button = foo; Also, QDialogButtonBox::addButton(QPushButton*) takes ownership, so the this parent isn't required | |
| 97 | While at it, use new connect syntax | |
| src/kfinddlg.h | ||
| 76 | I would prefer if you gave them proper names, like searchButton etc. | |
Comment Actions
- The includes are sorted now.
- The buttons renamed using "m_<name>Button" syntax.
- Spaces are added around the |.
- Declaration of the buttons rewritten to avoid the crash.
- The new syntax is used to connect signals and slots.
Comment Actions
One thing I noticed after this change is that the "Name" text field doesn't get automatically focused as I open KFind. Can you perhaps investigate this? :)