D21249: Test current filter before setting a new one

[David Faure]

dfaure added a comment.


  Yes, naming is hard because the method is dual-purposed ;-)
  
  If the method only tried to match (but not to set), then the naming would be much simpler.
  
     QString findMatchingFilter(....) const;
    
    if (!findMatchingFilter(...).isEmpty())
       return;
    
    foreach(....) {
        const QString filter = findMatchingFilter(....);
        if (!filter.isEmpty()) {
             setCurrentFilter(filter);
            return;
        }
    }
  
  > It "felt" strange, but I don't care much.
  
  I do care, because others will try to understand and possibly modify this code later, so it should not "feel strange".

INLINE COMMENTS

> dfaure wrote in kfilewidget.cpp:2495
> The added '|' isn't needed, is it?
> 
> str.left(-1) returns the whole string.

You wrote "done", but it's still there.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D21249

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190519/472b47f2/attachment-0001.html>