src/filewidgets/kfilewidget.cpp
1223	Whenever disconnect after that reset connection to QMetaObject::Connection cause disconnect empty connection is safe, double disconnect same one - does not.

jtamate added inline comments.Sep 7 2018, 11:27 AM

src/filewidgets/kfilewidget.cpp
438	I think it is using the url, the parameter to the lambda and _k_urlEntered(url); Changed & for this, d can't be used directly.
471–472	Because I didn't knew KActionCollection have it already! Done.
1223	I'm sorry but I don't understand your comment. m_connEditTextChanged is created in the constructor, line 585. Afterwards it is disconnected and reconnected in: setDummyHistoryEntry, removeDummyHistoryEntry and readRecentFiles.

anthonyfieroni added inline comments.Sep 7 2018, 11:35 AM

src/filewidgets/kfilewidget.cpp
1223	QObject::disconnect(m_connEditTextChanged); m_connEditTextChanged = QMetaObject::Connection();

jtamate marked an inline comment as done.Sep 7 2018, 12:00 PM

jtamate added inline comments.

src/filewidgets/kfilewidget.cpp
1223	I'm sorry, but I still don't get it. Doesn't connect/disconnect work more or less as new/delete? I mean, if m_connEditTextChanged is reconnected, can't it be disconnected safely?

Changed to 'Anonymous' connects and disconnects.

Even if the documentation say: use 0 as a wildcard, the compiler thinks it should be a nullptr

kfilewidget.cpp:1222:74: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]

bruns added a subscriber: bruns.Sep 7 2018, 5:00 PM

bruns added inline comments.

src/filewidgets/kfilewidget.cpp
1223	Me neither ... But me wonders if this should not use `{QSignalBlocker(locationEdit); ...}` instead. Also, the comment above is slightly wrong, there is no `setCurrentItem()` below.

Use nullptr instead of 0.
Remove the copied/pasted part of the cause of two dissconnects.

@bruns, as I'm not sure if the code emits other signals for lineEdit, I prefer to keep it as it was.

Fix a crash ,caused by a still connected signal, after running again the unittests.
kfilewidgettest still doesn't pass because QTest::qWaitForWindowActive fails for me.

Don't accept this revision until kfilewidgettest works flawlessly.

QTest::qWaitForWindowActive fails because I use kwin Focus stealing prevention High, therefore the windows doesn't become active until I click on them in the task bar or switch to them.
And qWaitForWindowExposed doesn't help because the window/widget needs to have the focus.

Is creating a README file and a kwin script to disable stealing prevention for kio tests an acceptable solution?

			Path	Packages
M			src/filewidgets/kfilewidget.h (21 lines)
M			src/filewidgets/kfilewidget.cpp (124 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	41145		Sep 7 2018, 10:06 AM	★	★
Diff 2	41148		Sep 7 2018, 11:03 AM	★	★
Diff 3	41150		Sep 7 2018, 12:23 PM	★	★
Diff 4	41246		Sep 9 2018, 9:03 AM	★	★
Diff 5	43106		Oct 8 2018, 7:41 AM	★	★

kfilewidget: convert connect syntax
Changes PlannedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 43106

src/filewidgets/kfilewidget.h

src/filewidgets/kfilewidget.cpp

kfilewidget: convert connect syntaxChanges PlannedPublicActions