Does using setPassword() not work for these tests?

See discussion about properies.

Why this sizeHint?

Please add a Q_PROPERTY for password, including passwordChanged() signal (NOTIFY method).

Applications should not have to deal with the passwordLineEdit() function in the regular use-case, but having the accessor function for special purpose is a good idea.

Either remove the 'p', or name it 'password'.

Surprised to see no K-prefix being used here? Why that?

Please add one and rename to KPasswordLineEdit:

• for consistency with the existing classes from kwidgetaddons/kde frameworks
• to protect against potential clashes, even if rare chance, with existing code linking to kwidgetaddons, which assumes using unprefixed/namespace-less names is safe in own code and might use that name for something else already.
• namespace-less class name lack any hint to origin, but rather used for project-internal code, so confuses code readers

Nope as setPassword not authorize to see icon (it's a security when we setPassword from apps you don't want to see it)

designer bug :)

manual ? :) nope it's designer which added it

Just for info LineEditUrlDropEventFilter has not K prefix
but ok I will rename it

Could you add this as comment to this line? it is not directly obvious, so would be good to store this knowledge directly in the code, so the next code reader gets why the code is like this.

Can be removed, given the QLineEdit include (for QLineEdit::EchoMode)

Thanks :) Yes, seems LineEditUrlDropEventFilter slipped in without somebody catching that.

When you do could you please also adapt PasswordLineEditPrivate? Even the test best would follow the naming pattern, so when scanning the dir with tests to find the matching test, it is found where expected (with default sorting by names).

Please also add API dox comment in front of the class, otherwise kapidox and ecm_add_qch will not pick up this class, given their doxygen settings.

@since should go to central class api dox comment (see other comment), or be behind the @param (see https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_Public_and_Protected_Members).

For a complete API this would also have isClearButtonEnabled() const here, and making this a clearButtonEnabled property.

With these properties e.g. integration in Qt Designer can be improved.

getter missing as well for balanced api, and should become a property as well.

Please add api dox for this signal. Which echo mode is triggered here? and can't it be reverted?

can be removed, class already forward-declared at the beginning, as normal non-nested one (as helpful for avoing visibility inheritance).

Make this a normal class KPasswordLineEditPrivate here, following fixes proposed above for the header.

change to KPasswordLineEditPrivate, so it is the forward declaration of the non-nested private class which KPasswordLineEdit should use.

better name this property clearButtonEnabled instead of clearButton, following the property name in qlinedit

Thanks for the docs. Hm, the name "echoModeTriggered" by itself is telling me that some "echomode" is triggered. While actually the signal tells that the echomode has been toggled/changed. So I would propose to rename it.

And ideally the signal would also carry the new echo mode, so the slot does not need to query again what the new mode is.
Just an suggestion while you expose this as public API, I see the old internal code was fine to just query again. But not really matching the typical Qt-style API that we all so like :)

Perhaps consider changing this to
void echoModeChanged(QLineEdit::EchoMode echoMode);

Remove this embedded class forward declaration -> results in leaked symbols due to visibility inherited.

rather extends QWidget now? unsure what exactly the consequences are

Again QWidget?

Should this not be rather lgpl2_+_?
see https://community.kde.org/Policies/Licensing_Policy

The consequence is that we don't see it in designer :)
So why not but it's better to show a lineedit no ?

I copied same license as knewpasswordwidget so I think it was ok.
So license is not the same in all kwidgetaddons ?

If that file is sorted (at least a bit), try to insert instead of append. Same for other places.

Can you double-check this change? To me, the echoMode() checks are now reversed compared to the original code.

I have not tested the code yet :)

Does this class need to be a QObject? If not, remove the QObject stuff.

Also needs the const QString &password argument.

Or if I misunderstand it, what is the difference between passwordChanged() and textChanged()? This class has no text() property, so having a textChanged() looks unexpected.

Nope it's ok as code is called after changing echomode
I created an autotest for testing it

Then please rename the slot to _k_echoModeToggled, otherwise is confusing.

Ping?

NOTIFY echoModeChanged

Document default value.

Please document the default value. If it is indeed QLineEdit::Password (which I would expect), remove the properties from the .ui files.

Ah, now I understand. Also, use the passed echoMode, instead of accessing lineEdit().

While you are at renaming slots: -> _k_passwordChanged

Please move the documentation for public functions above the documentation for internal functions (for those that read .h files, instead of apidox).

Ah wait, you are using the same slot for the verify widget, so maybe scratch that comment.

we can't as we use it in :
connect(toggleEchoModeAction, &QAction::triggered, this, &KPasswordLineEditPrivate::_k_echoModeToggled);

You made it a namespaced class again. Why?

Otherwise it failed to using Q_PRIVATE_SLOT...

That's fishy. I just checked a single class (KColumnResizer), and it has just "class KColumnResizerPrivate" (no namespace, no QObject), and still has a Q_PRIVATE_SLOT in its KColumnResizer class.

wierd it didn't want to build, but after a make clean it works...
next patch will come soon (12)

+1. Don't use nested private classes. Nested classes/structs inherit the symbols visibility.

Also see: https://phabricator.kde.org/D6927

PS: In times of Qt5 & lambda-connects, you don't need Q_PRIVATE_SLOT anymore, anyway:
Just remove the Q_PRIVATE_SLOT marker and ...:
connect(..., this, [this](...) { d->myPrivateSlot(...); });

Elvis, you wrote the "visibilityAction" stuff, right? If yes, needs to be mentioned here.

QLineEdit is included in the .h file, can be removed here.

Remove empty line.

Remove doubled empty line.

Does this need a protection for not emitting if the value did not actually change? I think it helps to prevent recursion.

Oh, that would make a nice "junior job" to get those cleaned out everywhere. Is removing a private slot considered API/ABI compatible?