D7011: Extract lineedit password

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> kpasswordlineedit.cpp:30
> +
> +class KPasswordLineEdit::KPasswordLineEditPrivate : public QObject
> +{

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

> kpasswordlineedit.h:25
> +class QAction;
> +class PasswordLineEditPrivate;
> +

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

> kpasswordlineedit.h:49
> +    Q_PROPERTY(QString password READ password WRITE setPassword NOTIFY passwordChanged)
> +    Q_PROPERTY(bool clearButton READ isClearButtonEnabled WRITE setClearButtonEnabled)
> +public:

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

> kpasswordlineedit.h:128
> +     */
> +    void echoModeTriggered();
> +    void passwordChanged();

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);`

> kpasswordlineedit.h:133
> +private:
> +    class KPasswordLineEditPrivate;
> +    KPasswordLineEditPrivate *const d;

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

REVISION DETAIL
  https://phabricator.kde.org/D7011

To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170801/dbd9981b/attachment-0001.html>