jglogowski marked an inline comment as done.
jglogowski added inline comments.
INLINE COMMENTS
> elvisangelaccio wrote in kfilewidget.cpp:132
> Nitpick: I'd make `filter` the first parameter. And we usually don't use
> `const boll` in signatures, but just `bool`.
>
> `bUpdate` doesn't tell the reader what this variable is used for. How about
> calling it `updateCurrentFilter` or similar?
Kind of Done.
> dfaure wrote in kfilewidget.cpp:2463
> Is the bUpdate bool necessary?
>
> Without it, we'd call setCurrentFilter(currentFilter()) which should be a
> no-op, right?
>
> Alternatively, the method could return a QString, and the (second) caller
> could call setCurrentFilter.
>
> I just don't like a method that's sometimes a getter and sometimes a setter
> (basically).
> Is the bUpdate bool necessary?
void KFileFilterCombo::setCurrentFilter(const QString &filter)
{
setCurrentIndex(d->m_filters.indexOf(filter));
emit filterChanged();
}
filterChanged will unconditionally start the "cycle", which will set the wrong
filter.
Wanted to keep my changes more local.
> Alternatively, the method could return a QString, and the (second) caller
> could call setCurrentFilter.
> I just don't like a method that's sometimes a getter and sometimes a setter
> (basically).
Also had that. It "felt" strange, but I don't care much.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D21249
To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh,
bruns
