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

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> index.docbook:2141
> +<term><guimenuitem>Set Show Hidden Folders</guimenuitem></term>
> +<listitem><para>Toggle whether hidden folders should be shown in the treeview.</para></listitem>
> +</varlistentry>

hidden folders (whose name starts with a dot)

That's what this is all about, right?

> sidebar_widget.cpp:339
> +    bool canToggle = currentButtonInfo().canToggleShowHiddenFolders;
> +    if (canToggle) {
> +        bool newToggleState = !currentButtonInfo().showHiddenFolders;

This should be an assert instead, you don't create the action if canToggle... is false.

Q_ASSERT(currentButtonInfo().canToggleShowHiddenFolders);

> sidebar_widget.cpp:543
>          buttonInfo.configOpen = configGroup.readEntry("Open", false);
> +        buttonInfo.canToggleShowHiddenFolders = ! configGroup.readEntry("ShowHiddenFolders", QString()).isEmpty();
> +        buttonInfo.showHiddenFolders = configGroup.readEntry("ShowHiddenFolders", false);

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.

> sidebar_widget.cpp:580
> +                    if (currentButtonInfo().showHiddenFolders) {
> +                        buttonPopup->addAction(QIcon::fromTheme("fileopen"), i18n("Hide Hidden Folders..."), this, SLOT(slotToggleShowHiddenFolders())); // NOTE: does it matter that it toggles? Race conditions?
> +                    }

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

> tree_module.cpp:77
> +    // treeView->header()->setSectionResizeMode(0, QHeaderView::ResizeToContents);
> +    // treeView->header()->setSectionResizeMode(0, QHeaderView::Stretch);
> +    //treeView->header()->setStretchLastSection(false);

Please don't add dead code.

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/20200801/56b0f56f/attachment.htm>