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
