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
