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