D29871: Enable option to show hidden folders in sidebar, plus small sidebar code fixes (eg: scrollbar display)

[Raphael Rosch]

rrosch added inline comments.

INLINE COMMENTS

> dfaure wrote in index.docbook:2141
> hidden folders (whose name starts with a dot)
> 
> That's what this is all about, right?

Do you mean that I should edit the text to add the stuff in the parentheses?

> dfaure wrote in sidebar_widget.cpp:339
> This should be an assert instead, you don't create the action if canToggle... is false.
> 
> Q_ASSERT(currentButtonInfo().canToggleShowHiddenFolders);

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?

> dfaure wrote in sidebar_widget.cpp:543
> Why not enable the feature for *any* button that ends up creating a KonqSideBarTreeModule?
> 
> 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.
> 
> 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.
> And since it's available for any user of KDirModel, I'd just make it depend on the module being used.

Sure, I can make it available for any KonqSideBarTreeModule button if you want. Will do in the next revision.

> dfaure wrote in sidebar_widget.cpp:580
> "Hide Hidden" feels weird.
> 
> Please copy Dolphin. It uses a toggle action "Show Hidden Folders", the text doesn't change, just the checkmark in front.
> 
> QWidget is all single threaded, I wonder which race condition you have in mind here, please explain.

Ok, I'm going to look up how to do the checkbox. Hopefully it's straightforward and doesn't require too much code.

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.

> dfaure wrote in tree_module.cpp:77
> Please don't add dead code.

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.

REPOSITORY
  R226 Konqueror

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

To: rrosch, dfaure
Cc: kde-doc-english, gennad, fbampaloukas, skadinna
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-doc-english/attachments/20200805/65cf167b/attachment.htm>