D7011: Extract lineedit password

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> mlaurent wrote in knewpasswordwidgettest.cpp:63
> Nope as setPassword not authorize to see icon (it's a security when we setPassword from apps you don't want to see 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.

> passwordlineedit.h:24
> +#include <kwidgetsaddons_export.h>
> +class QLineEdit;
> +class QAction;

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

> mlaurent wrote in passwordlineedit.h:27
> Just for info LineEditUrlDropEventFilter  has not K prefix
> but ok I will rename it

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).

> passwordlineedit.h:27
> +class PasswordLineEditPrivate;
> +class KWIDGETSADDONS_EXPORT PasswordLineEdit : public QWidget
> +{

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.

> passwordlineedit.h:33
> +     * Constructs a lineedit password widget.
> +     * @since 5.37
> +     *

@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).

> passwordlineedit.h:61
> +     */
> +    void setClearButtonEnabled(bool clear);
> +

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.

> passwordlineedit.h:66
> +     */
> +    void setEchoMode(QLineEdit::EchoMode mode);
> +    /**

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

> passwordlineedit.h:94
> +Q_SIGNALS:
> +    void echoModeTriggered();
> +

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

> passwordlineedit.h:97
> +private:
> +    class PasswordLineEditPrivate;
> +    PasswordLineEditPrivate *const d;

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

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/20170731/08755630/attachment.html>