D29871: Enable option to show hidden folders in sidebar, plus small sidebar code fixes (eg: scrollbar display)
Raphael Rosch
noreply at phabricator.kde.org
Tue Aug 11 07:11:45 BST 2020
rrosch added a comment.
In D29871#676166 <https://phabricator.kde.org/D29871#676166>, @dfaure wrote:
> In D29871#676150 <https://phabricator.kde.org/D29871#676150>, @rrosch wrote:
>
> > I didn't have an assert before though, should I replace the if statement with a Q_ASSERT then?
>
>
> Yes I still believe you should.
>
> > I thought that was just to catch errors
>
> Yes, it is. If everything works as intended, this method will never be called with canToggleShowHiddenFolders == false.
> And `if()` makes people think that it can, which is just not true.
>
> > but since this is just an option
>
> What is an option?
>
> > (and hence shouldn't cause problems when false)
>
> Confusing code is a problem.
Ohhh.. now I see what you mean, I had mentally lost track of some of the revision changes, and forgot that the option to show the context menu option was already being selected in a previous part of the code (as opposed to here).
I have now implemented the changes you suggested. Hopefully it's all ok now.
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/20200811/0d3d81c2/attachment.htm>
More information about the kde-doc-english
mailing list