<table><tr><td style="">rrosch 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/D29871">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/D29871#inline-172030">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">index.docbook:2141</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">hidden folders (whose name starts with a dot)</p>

<p style="padding: 0; margin: 8px;">That's what this is all about, right?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Do you mean that I should edit the text to add the stuff in the parentheses?</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/D29871#inline-172027">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sidebar_widget.cpp:339</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This should be an assert instead, you don't create the action if canToggle... is false.</p>

<p style="padding: 0; margin: 8px;">Q_ASSERT(currentButtonInfo().canToggleShowHiddenFolders);</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I haven't used Q_ASSERT before, and when I looked it up just now it says that all it does is print a warning message if the boolean is false. Is that what you mean?</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/D29871#inline-172031">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sidebar_widget.cpp:543</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Why not enable the feature for *any* button that ends up creating a KonqSideBarTreeModule?</p>

<p style="padding: 0; margin: 8px;">Otherwise, as a user, I do "RMB / Add New / Tree Sidebar Module", configure its URL to /tmp or whatever and the feature is broken there.</p>

<p style="padding: 0; margin: 8px;">By not making the on/off dependent on the desktop file entry, maybe one day we can change the storing of the on/off status to be per-directory. Availability of the feature and on/off storage should be separate.<br />
And since it's available for any user of KDirModel, I'd just make it depend on the module being used.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Sure, I can make it available for any KonqSideBarTreeModule button if you want. Will do in the next revision.</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/D29871#inline-172029">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">sidebar_widget.cpp:580</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">"Hide Hidden" feels weird.</p>

<p style="padding: 0; margin: 8px;">Please copy Dolphin. It uses a toggle action "Show Hidden Folders", the text doesn't change, just the checkmark in front.</p>

<p style="padding: 0; margin: 8px;">QWidget is all single threaded, I wonder which race condition you have in mind here, please explain.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Ok, I'm going to look up how to do the checkbox. Hopefully it's straightforward and doesn't require too much code.</p>

<p style="padding: 0; margin: 8px;">The race condition I had in mind was when you have two windows open and you change the toggle in one, and then again in the other. Stefano says that his Konq does the change live, but that doesn't seem to be working for me (not sure if my Konq, KF5 or Qt is to blame), but in any case if the change happens live, then that solves it and I guess there is no race condition.</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/D29871#inline-172028">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">tree_module.cpp:77</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Please don't add dead code.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yeah this was by mistake, I somehow missed it while doing code cleanup and only realized it was still in after I had already submitted the new commit.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R226 Konqueror</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D29871">https://phabricator.kde.org/D29871</a></div></div><br /><div><strong>To: </strong>rrosch, dfaure<br /><strong>Cc: </strong>kde-doc-english, gennad, fbampaloukas, skadinna<br /></div>