Review Request 120144: Equal entries imply existing

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

Re: Review Request 120144: Equal entries imply existing

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-devel@kde.org https

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,

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=58193 REVISION DETAIL https://phabricator.kde.org/D21249

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

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

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=58317 REVISION DETAIL https://phabricator.kde.org/D21249 AFFECTED FILES

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

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

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

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

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.

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=58394 REVISION DETAIL https://phabricator.kde.org/D21249 AFFECTED FILES

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