jglogowski added inline comments.
INLINE COMMENTS
> dfaure wrote in kfilewidgettest.cpp:88
> ... use setUrls *to* auto-select ODT filter?
>
> ("to" is missing)
No. The first line is missing a point or semicolon, if you read the comment
like a sentence. I had two independent comments in mind. Or some kind of
annotation (*) to make it look more like a list.
The first line is a general comment, the 2nd describes the actual test case.. I
know the blocking is documented, but it's still not expected behavior. At least
I was puzzled and even read the code before the API doc, as I suspected a bug
somewhere (that didn't exclude my code).
> kfilewidgettest.cpp:94
> + // select 2nd duplicate XML filter
> + // see bug 407642
> + fw.filterWidget()->setCurrentFilter("*.xml *.b|DocBook (.xml)");
From your POV this probably has the same problem then the first comment. I'll
change it to:
// select 2nd duplicate XML filter (see bug 407642)
> dfaure wrote in kfilewidget.cpp:2456
> This method could (and should) be marked as const, now.
Will do.
> dfaure wrote in kfilewidget.cpp:2492
> I don't really understand what this comment is doing here, it seems unrelated
> to the next line of code. Is it just a longer version of the comment on line
> 2499?
Kind of a reverse comment of line 2499. My original code tested for '*' and I
found it none-obvious to omit that match test here. I even tested both variants
with the bug program to be sure. The comment is a little disturbing, as there
wasn't any code handling '*' yet. I'll change it to:
// accept any match to honor the user's selection; see later code handling the
"*" match
Or I drop it. I would keep it. Other suggestions?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D21249
To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh,
bruns
