<table><tr><td style="">kossebau added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D7011" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28630" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mlaurent</span> wrote in <span style="color: #4b4d51; font-weight: bold;">knewpasswordwidgettest.cpp:63</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Nope as setPassword not authorize to see icon (it's a security when we setPassword from apps you don't want to see it)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28659" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:24</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf"><kwidgetsaddons_export.h></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">class</span> <span class="n">QLineEdit</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">class</span> <span class="n">QAction</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Can be removed, given the QLineEdit include (for QLineEdit::EchoMode)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28633" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mlaurent</span> wrote in <span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:27</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Just for info LineEditUrlDropEventFilter has not K prefix<br />
but ok I will rename it</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Thanks :) Yes, seems LineEditUrlDropEventFilter slipped in without somebody catching that.</p>
<p style="padding: 0; margin: 8px;">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).</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28654" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:27</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">class</span> <span class="n">PasswordLineEditPrivate</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">class</span> <span class="n">KWIDGETSADDONS_EXPORT</span> <span style="color: #a0a000">PasswordLineEdit</span> <span class="p">:</span> <span class="n">public</span> <span class="n">QWidget</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28655" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:33</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Constructs a lineedit password widget.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * @since 5.37</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><span class="phabricator-remarkup-mention-unknown">@since</span> should go to central class api dox comment (see other comment), or be behind the <span class="phabricator-remarkup-mention-unknown">@param</span> (see <a href="https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_Public_and_Protected_Members" class="remarkup-link" target="_blank" rel="noreferrer">https://community.kde.org/Frameworks/Frameworks_Documentation_Policy#Document_Public_and_Protected_Members</a>).</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28653" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:61</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">void</span> <span style="color: #004012">setClearButtonEnabled</span><span class="p">(</span><span style="color: #aa4000">bool</span> <span class="n">clear</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">For a complete API this would also have <tt style="background: #ebebeb; font-size: 13px;">isClearButtonEnabled() const</tt> here, and making this a <tt style="background: #ebebeb; font-size: 13px;">clearButtonEnabled</tt> property.</p>
<p style="padding: 0; margin: 8px;">With these properties e.g. integration in Qt Designer can be improved.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28656" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:66</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">void</span> <span style="color: #004012">setEchoMode</span><span class="p">(</span><span class="n">QLineEdit</span><span style="color: #aa2211">::</span><span class="n">EchoMode</span> <span class="n">mode</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">/**</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">getter missing as well for balanced api, and should become a property as well.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28660" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:94</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #a0a000">Q_SIGNALS</span><span class="p">:</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">void</span> <span class="n">echoModeTriggered</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please add api dox for this signal. Which echo mode is triggered here? and can't it be reverted?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7011#inline-28657" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">passwordlineedit.h:97</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #a0a000">private</span><span class="p">:</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">class</span> <span class="n">PasswordLineEditPrivate</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">PasswordLineEditPrivate</span> <span style="color: #aa2211">*</span><span style="color: #aa4000">const</span> <span class="n">d</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">can be removed, class already forward-declared at the beginning, as normal non-nested one (as helpful for avoing visibility inheritance).</p></div></div></div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7011" rel="noreferrer">https://phabricator.kde.org/D7011</a></div></div><br /><div><strong>To: </strong>mlaurent, cfeck, dfaure, elvisangelaccio<br /><strong>Cc: </strong>kossebau, elvisangelaccio, Frameworks<br /></div>