D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58427. jglogowski added a comment. - switch declaration KFileWidgetPrivate::findMatchingFilter from bool to QString … again Not sure how that exactly slipped in :-( REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58394. jglogowski added a comment. - const findMatchingFilter - amend comments REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21249?vs=58345&id=58394 REVISION DETAIL https://phabricator.kde.org/D21249 AFFECTED FILE

D21249: Test current filter before setting a new one

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. O

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58345. jglogowski added a comment. - Merge test/kfilewidgettest_filter.cpp into autotests/kfilewidgettest.cpp - Swap QCOMPARE parameters to match actual + expected output on failure - Always test filter and file name Technically the '*' filter j

D21249: Test current filter before setting a new one

jglogowski added a comment. In D21249#467037 , @dfaure wrote: > A unittest addition would be good, too. Done In D21249#467073 , @dfaure wrote: > Yes, naming is hard because the method is

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58317. jglogowski added a comment. Readd dropped lines from tests/CMakeLists.txt REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21249?vs=58316&id=58317 REVISION DETAIL https://phabricator.kde.org/D21249 AFFECTED FILE

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58316. jglogowski added a comment. - Return a QString from findMatchingFilter and handle setCurrentFilter in main function - Add filter unit test For a manual test I added a "Raw (*)" filter to the bug test program. After playing with it, I dec

D21249: Test current filter before setting a new one

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

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58291. jglogowski added a comment. Changes: - Dropped the duplicate comment in matchFilter (not sure if it makes sense at all) - Replace bool param with enum class to improve readability - should have done this from the start - Drop const from

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58215. jglogowski added a comment. Hope this compiles now. Since I can't test the rebased patch, I hope that is the last update. The original working version is based on v5.44. See my previous comment. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D21249: Test current filter before setting a new one

jglogowski added a reviewer: Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21249 To: jglogowski, #frameworks Cc: michaelweghorn, kde-frameworks-devel, michaelh, ngraham, bruns

D21249: Test current filter before setting a new one

jglogowski updated this revision to Diff 58193. jglogowski added a comment. Readd dropped QLatin1Char and use them in new code too. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21249?vs=58191&id=58193 REVISION DETAIL https://phabricator.kde.org/D21249 AFF

D21249: Test current filter before setting a new one

jglogowski planned changes to this revision. jglogowski added a comment. This patch was developed on KIO v5.44 (git tag) in an Ubuntu 18.04 chroot (because that's my LibreOffice development environment) and rebased on master. The test-program was run on Debian Buster via LD_PRELOAD=./git_ki

D21249: Test current filter before setting a new one

jglogowski created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jglogowski requested review of this revision. REVISION SUMMARY If KFileWidget's filter list has two matching filters for an extension, it will always select the first filter,

Re: Review Request 120144: Equal entries imply existing

(both have the same inode). This was just tested and fixed in our KDE4 kfile handler and konqueror, which have the same problem (KDE 4.12 on Kubuntu 12.04). Thanks, Jan-Marek Glogowski ___ Kde-frameworks-devel mailing list Kde-frameworks

Review Request 120144: Equal entries imply existing

nqueror, which have the same problem (KDE 4.12 on Kubuntu 12.04). Thanks, Jan-Marek Glogowski ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel