<table><tr><td style="">tuxxi 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/D18939">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/D18939#inline-105595">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ngraham</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:30</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Unrelated change.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">IMHO it looks cleaner to separate includes into categories, which is what I did here.</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/D18939#inline-105597">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ngraham</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:141</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Since the placeholder text says "Search..." we should probably not use the word "Filter" in the tooltip. Also "Type to" isa not really necessary since that's the only thing you can do with a search field. :)</p>
<p style="padding: 0; margin: 8px;">How about something like "Search through the list of applications below"?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I copied the tooltip text from KOpenWithDialog, but I'll change this one. Perhaps we can change it 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/D18939#inline-105598">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ngraham</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:143</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">No need to use <tt style="background: #ebebeb; font-size: 13px;">auto</tt> here</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm using <tt style="background: #ebebeb; font-size: 13px;">auto</tt> to avoid duplicating the type name when using <tt style="background: #ebebeb; font-size: 13px;">new</tt> since the type is right there; its a habit I got into since clang-tidy recommends it. I couldn't find anything in the style guides prohibiting this. If you _really_ want, I can stop using <tt style="background: #ebebeb; font-size: 13px;">auto</tt>.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R103 KMenu Editor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18939">https://phabricator.kde.org/D18939</a></div></div><br /><div><strong>To: </strong>tuxxi, ngraham, Plasma, cfeck<br /><strong>Cc: </strong>ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>