D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

ahmadsamir added a comment. The user typing 'arc' then pressing Enter, and getting the file opened with wireshark is broken behaviour form the user's POV, that's the crux of the bug report in question. That combobox uses KCompletion and not having the up/down arrow keys navigate

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

ahmadsamir added a comment. In D24951#554138 , @ngraham wrote: > Why wouldn't you want the first result chosen when there's more than one result? If you enter a search term and get (say) 2 results and the one you want to select is the first

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

ahmadsamir created this revision. ahmadsamir added a reviewer: dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY If the user types some text in the lineEdit and there's more than one match in the model view, don't automatically select

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

ahmadsamir added a comment. Is cipherDigestMethod() (method @ line 96) needed? the cipher name is inserted in sslMetaData a couple of lines after that. Using some test code: QList list = QSslConfiguration::supportedCiphers(); for (QSslCipher c : list) { qInfo() << c.name();

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

ahmadsamir added a dependent revision: D24929: [pop3 kioslave] Port KTcpSocket (deprecated) to QSslSocket. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24928 To: ahmadsamir, dfaure, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24873: [http kio slave] use QSslSocket instead of KTcpSocket (deprecated)

ahmadsamir updated this revision to Diff 68702. ahmadsamir edited the summary of this revision. ahmadsamir added a dependency: D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket. ahmadsamir added a comment. Depends on D24928 REPOSITORY

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

ahmadsamir added a dependent revision: D24873: [http kio slave] use QSslSocket instead of KTcpSocket (deprecated). REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24928 To: ahmadsamir, dfaure, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24928: [TcpSlaveBase] port from KTcpSocket (deprecated) to QSslSocket

ahmadsamir created this revision. ahmadsamir added reviewers: dfaure, vkrause. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY Also add a method similar to KSslCipher::digestMethod() to TcpSlaveBase. REPOSITORY R241 KIO BRANCH

D24873: [http kio slave] use QSslSocket instead of KTcpSocket (deprecated)

ahmadsamir added a comment. In D24873#552795 , @vkrause wrote: > In D24873#552697 , @ahmadsamir wrote: > > > In D24873#552410 , @vkrause wrote: > > >

D24873: [http kio slave] use QSslSocket instead of KTcpSocket (deprecated)

ahmadsamir added a comment. In D24873#552410 , @vkrause wrote: > This will compile, but it will only work correctly once TCPSlaveBase has been ported to QSslSocket too. If you say so. Anyway, I've started porting TcpSlaveBase. :)

D24873: [http kio slave] use QSslSocket instead of KTcpSocket (deprecated)

ahmadsamir created this revision. ahmadsamir added reviewers: dfaure, vkrause. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN make && ctest REPOSITORY R241 KIO BRANCH ahmad/kio_http-ktcpsocket (branched from master) REVISION DETAIL

D24828: KRun: don't override user preferred app when opening local *.*html and co. files

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY KRun unconditionally opened files with mimetype text/html (and co.) with the browser application set in

D24350: [src/kpac/*] replace deprecated foreach with range for

This revision was automatically updated to reflect the committed changes. Closed by commit R241:103e13c2765e: [src/kpac/*] replace deprecated foreach with range for (authored by ahmadsamir). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24350?vs=68395=68419

D24350: [src/kpac/*] replace deprecated foreach with range for

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in script.cpp:157 > Why not just change this one to return `const &` and then use it in all > range-fors? > > [note that returning a const ref is bad in public API, but this is an > internal method so we can always change it

D24350: [src/kpac/*] replace deprecated foreach with range for

ahmadsamir updated this revision to Diff 68395. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. In private API we can have a method return a const &, as it can be changed later without side-effects. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D24350: [src/kpac/*] replace deprecated foreach with range for

ahmadsamir added a comment. Ping... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24350 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24793: [Konqueror] Remove KonqView::setActiveComponent()

ahmadsamir created this revision. ahmadsamir added reviewers: dfaure, stefanocrocco. ahmadsamir requested review of this revision. REVISION SUMMARY It's mostly redundant since KF5 port; the remaining bit m_activityResourceInstance->notifyFocusedIn() call can be moved to

D24597: [Konversation] Port from deprecated KTcpSocket to QSslSocket

This revision was automatically updated to reflect the committed changes. Closed by commit R7:458822190452: [Konversation] Port from deprecated KTcpSocket to QSslSocket (authored by ahmadsamir). REPOSITORY R7 Konversation CHANGES SINCE LAST UPDATE

D24770: KLocale api docs: make it easier to find how to port code away from it

This revision was automatically updated to reflect the committed changes. Closed by commit R239:556f102af097: KLocale api docs: make it easier to find how to port code away from it (authored by ahmadsamir). REPOSITORY R239 KDELibs4Support CHANGES SINCE LAST UPDATE

D24770: KLocale api docs: make it easier to find how to port code away from it

ahmadsamir created this revision. ahmadsamir added a reviewer: Frameworks. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REPOSITORY R239 KDELibs4Support BRANCH ahmad/klocale-notes (branched from master) REVISION DETAIL https://phabricator.kde.org/D24770

D24746: [kpropertiesdialog] adjust ui for root directory

ahmadsamir added inline comments. INLINE COMMENTS > kpropertiesdialog.cpp:1172 > > -l = new QLabel(i18n("Mounted on:"), d->m_frame); > -grid->addWidget(l, curRow, 0, Qt::AlignRight); > +// only show for non-root folders because '/' is obviously

D24725: Fix grammar

This revision was automatically updated to reflect the committed changes. Closed by commit R244:b539685e9495: Fix grammar (authored by ahmadsamir). REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24725?vs=68118=68123 REVISION DETAIL

D24725: GIT_SILENT Fix grammar

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, mpyne. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REPOSITORY R244 KCoreAddons BRANCH text-fix (branched from master) REVISION DETAIL https://phabricator.kde.org/D24725

D24672: GIT_SILENT Run uncrustify-kf5 on the whole tree

ahmadsamir abandoned this revision. REPOSITORY R374 KolourPaint REVISION DETAIL https://phabricator.kde.org/D24672 To: ahmadsamir, kde-frameworks-devel, mkoller Cc: cullmann, ognarb, kde-frameworks-devel

D24672: GIT_SILENT Run uncrustify-kf5 on the whole tree

ahmadsamir added a comment. In D24672#547744 , @mkoller wrote: > why ? > I don't like lots of these changes. Because it doesn't follow the KDE coding style, AFAICS (and changing it to use that common style will make it easier for

D24675: Deprecate the KTcpSocket-based variant of SslUi::askIgnoreSslErrors

ahmadsamir added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24675 To: vkrause, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24672: GIT_SILENT Run uncrustify-kf5 on the whole tree

ahmadsamir added a comment. T11214 is still being discussed, and we can always run clang-format later on. And uncrustify-kf5 has been used before on some of the KDE repos, so it's sort of have been fine-tuned a bit to KDE. Anyway, I don't mind

D24672: GIT_SILENT Run uncrustify-kf5 on the whole tree

ahmadsamir added a task: T11576: Port kolourpaint away from kdelibs4support. REPOSITORY R374 KolourPaint REVISION DETAIL https://phabricator.kde.org/D24672 To: ahmadsamir, kde-frameworks-devel Cc: kde-frameworks-devel

D24672: GIT_SILENT Run uncrustify-kf5 on the whole tree

ahmadsamir added a reviewer: kde-frameworks-devel. ahmadsamir added a subscriber: kde-frameworks-devel. REPOSITORY R374 KolourPaint REVISION DETAIL https://phabricator.kde.org/D24672 To: ahmadsamir, kde-frameworks-devel Cc: kde-frameworks-devel

D24631: exclude ioslaves directory from api docs

ahmadsamir added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24631 To: sitter, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D19504: Extend KHtmlView::print() to use a predefined QPrinter instance

ahmadsamir added reviewers: dfaure, vkrause. REPOSITORY R313 KHtml REVISION DETAIL https://phabricator.kde.org/D19504 To: habacker, dfaure, vkrause Cc: aacid, tbaumgart, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24588: [SslUi] Add api docs for askIgnoreSslErrors()

This revision was automatically updated to reflect the committed changes. Closed by commit R241:c38591b9d2a9: [SslUi] Add api docs for askIgnoreSslErrors() (authored by ahmadsamir). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24588?vs=67834=67892 REVISION

D24597: [Konversation] Port from deprecated KTcpSocket to QSslSocket

ahmadsamir updated this revision to Diff 67835. ahmadsamir retitled this revision from "Port from deprecated KTcpSocket to QSslSocket" to "[Konversation] Port from deprecated KTcpSocket to QSslSocket". ahmadsamir added a comment. Better commit message REPOSITORY R7 Konversation CHANGES

D24588: [SslUi] Add api docs for askIgnoreSslErrors()

ahmadsamir updated this revision to Diff 67834. ahmadsamir retitled this revision from "[SslUi] Overload askIgnoreSslErrors() to take a QSslSocket" to "[SslUi] Add api docs for askIgnoreSslErrors()". ahmadsamir added a comment. Since I wrote the docs bit anyway REPOSITORY R241 KIO CHANGES

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir removed a task: T11577: Port Konversation away from KTcpSocket. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24588 To: ahmadsamir, dfaure, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24597: Port from deprecated KTcpSocket to QSslSocket

ahmadsamir updated this revision to Diff 67833. ahmadsamir edited the summary of this revision. ahmadsamir removed reviewers: hein, mlaurent. ahmadsamir added a comment. Use KSslErrorUiData directly when calling askIgnoreSslErrors REPOSITORY R7 Konversation CHANGES SINCE LAST UPDATE

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir added a comment. In D24588#546367 , @vkrause wrote: > In D24588#546229 , @dfaure wrote: > > > Hmm, why not just do `KSslErrorUiData uiData(socket);` in the caller, or even just

D24603: Port away from deprecated KDELibs4Support

This revision was automatically updated to reflect the committed changes. Closed by commit R103:e3b8910670e3: Port away from deprecated KDELibs4Support (authored by ahmadsamir). REPOSITORY R103 KMenu Editor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24603?vs=67820=67831

D24603: Port away from deprecated KDELibs4Support

ahmadsamir added a comment. F7582001: kmenuedit-vs-dolphin-settings-dialog.png Looking at kmenuedit and dolphin settings dialog, the margin and spacing look comparable, so those setContentMargins and setSpacing calls aren't needed any more.

D24603: Port away from deprecated KDELibs4Support

ahmadsamir added a task: T11566: Port kmenuedit away from kdelibs4support. REPOSITORY R103 KMenu Editor REVISION DETAIL https://phabricator.kde.org/D24603 To: ahmadsamir, mlaurent Cc: plasma-devel, kde-frameworks-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen,

D24603: Port away from deprecated KDELibs4Support

ahmadsamir created this revision. ahmadsamir added a reviewer: mlaurent. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ahmadsamir requested review of this revision. REVISION SUMMARY kmenuedit seems to work, adding new submenus and items REPOSITORY R103 KMenu Editor

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir updated this revision to Diff 67818. ahmadsamir added a comment. @since should actually be in the future REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24588?vs=67817=67818 BRANCH ahmad/ktcpsocket-qsslsocket (branched from master) REVISION

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir updated this revision to Diff 67817. ahmadsamir added a comment. Api docs... REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24588?vs=67778=67817 BRANCH ahmad/ktcpsocket-qsslsocket (branched from master) REVISION DETAIL

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir added a comment. In D24588#546229 , @dfaure wrote: > Hmm, why not just do `KSslErrorUiData uiData(socket);` in the caller, or even just `askIgnoreSslErrors(KSslErrorUiData(socket), storedRules)`. > Don't know, just wondering (not

D24372: Compile without deprecated foreach

This revision was automatically updated to reflect the committed changes. Closed by commit R241:7c0402ed9fd7: Compile without deprecated foreach (authored by ahmadsamir). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24372?vs=67804=67808 REVISION DETAIL

D24372: Compile without deprecated foreach

ahmadsamir added a comment. I had to fix one conflict when rebasing, and basically D24419 ate the foreach src/core/ksslcertificatemanager.cpp@line 128 (by returning the result of calling contains() on the container, neater :)). REPOSITORY R241 KIO

D24372: Compile without deprecated foreach

ahmadsamir updated this revision to Diff 67804. ahmadsamir added a comment. Rebase REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24372?vs=67510=67804 BRANCH arcpatch-D24372 (branched from master) REVISION DETAIL https://phabricator.kde.org/D24372

D24597: Port from deprecated KTcpSocket to QSslSocket

ahmadsamir added a task: T11577: Port Konversation away from KTcpSocket. REPOSITORY R7 Konversation REVISION DETAIL https://phabricator.kde.org/D24597 To: ahmadsamir, #konversation, hein, mlaurent Cc: kde-frameworks-devel, #konversation

D24597: Port from deprecated KTcpSocket to QSslSocket

ahmadsamir added reviewers: hein, mlaurent. ahmadsamir added subscribers: Konversation, kde-frameworks-devel. REPOSITORY R7 Konversation REVISION DETAIL https://phabricator.kde.org/D24597 To: ahmadsamir, #konversation, hein, mlaurent Cc: kde-frameworks-devel, #konversation

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir added a dependent revision: D24597: Port from deprecated KTcpSocket to QSslSocket. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24588 To: ahmadsamir, dfaure, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir added a task: T11577: Port Konversation away from KTcpSocket. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24588 To: ahmadsamir, dfaure, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24588: [SslUi] Overload askIgnoreSslErrors() to take a QSslSocket

ahmadsamir created this revision. ahmadsamir added reviewers: dfaure, vkrause. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY This is needed as part of porting Koversation to use QSslSocket instead of KTcpSocket. TEST PLAN The code

D24372: Compile without deprecated foreach

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in job.cpp:181 > You need a local const var to hold the result of the subjobs() method call. > > (repeats) IIUC, subjobs() returns a const QList &, do we still need a local const var?

D24372: Compile without deprecated foreach

ahmadsamir updated this revision to Diff 67510. ahmadsamir marked 7 inline comments as done. ahmadsamir added a comment. - User more descriptive var names other than list2 - qAsConst isn't needed if the method is const and the container is a member var REPOSITORY R241 KIO CHANGES SINCE

D24463: Treat "application/x-ms-dos-executable" as executable on all platforms

ahmadsamir added a reviewer: dfaure. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D24463 To: guoyunhe, #frameworks, ngraham, dfaure Cc: dfaure, ngraham, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir added a comment. In D23902#542046 , @dfaure wrote: > In D23902#542037 , @ahmadsamir wrote: > > > In D23902#542026 , @dfaure wrote: > > > >

D24308: [DataProtocol] compile without implicit coversion from ascii

This revision was automatically updated to reflect the committed changes. Closed by commit R241:3b98d91436f5: [DataProtocol] compile without implicit coversion from ascii (authored by ahmadsamir). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir added a comment. In D23902#542026 , @dfaure wrote: > For the record, JFBastien was actually wrong. Calling .begin() on a const return value does call the const overload. Testcase http://www.davidfaure.fr/kde/const_retval.cpp

D24372: Compile without deprecated foreach

ahmadsamir created this revision. ahmadsamir added a reviewer: dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY src/core/* src/urifilters/* src/widgets/* TEST PLAN make && ctest REPOSITORY R241 KIO BRANCH

D24350: [src/kpac/*] replace deprecated foreach with range for

ahmadsamir created this revision. ahmadsamir added a reviewer: dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY Add convience function that returns const QList&, since that list is iterated over in several places in the code. TEST

D24349: More (and last) fixes to compile without implicit conversion from ASCII/ByteArray

ahmadsamir created this revision. ahmadsamir added reviewers: dfaure, kossebau. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN make && ctest REPOSITORY R241 KIO BRANCH ahmad/ascii-cast2 (branched from master) REVISION DETAIL

D24308: [DataProtocol] compile without implicit coversion from ascii

ahmadsamir updated this revision to Diff 67125. ahmadsamir added a comment. Make it faster by comparing char's directly REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24308?vs=67062=67125 BRANCH ahmad/compile-without-ascii-cast (branched from master)

D24308: [DataProtocol] compile without implicit coversion from ascii

ahmadsamir created this revision. ahmadsamir added a reviewer: dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN make && ctest REPOSITORY R241 KIO BRANCH ahmad/compile-without-ascii-cast (branched from master) REVISION DETAIL

D24220: [src/ioslaves/*] replace deprecated foreach with range for

ahmadsamir added a comment. (Please commit this one too). REPOSITORY R241 KIO BRANCH ahmad/foreach-ioslaves2 (branched from master) REVISION DETAIL https://phabricator.kde.org/D24220 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham,

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir added a comment. (Reminder: I don't have a dev account so you'll have to commit the patches :)). REPOSITORY R241 KIO BRANCH ahmad/foreach-filewidgets (branched from master) REVISION DETAIL https://phabricator.kde.org/D24171 To: ahmadsamir, kde-frameworks-devel, dfaure Cc:

D24262: Modernize code: use range-based loops & algorithms in more places

ahmadsamir added a comment. @kossebau: did you try the C++ Standard (working draft): https://isocpp.org/blog/2013/10/n3797-working-draft-standard-for-programming-language-c-stefanus-du-toit This ^ one is circa 2012. All the sources used to generate the C++ Standard drafts are

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir updated this revision to Diff 67008. ahmadsamir added a comment. Don't iterate over the temp. container returned by qhash.keys(), rather over the qhash itself REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24171?vs=67007=67008 BRANCH

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir updated this revision to Diff 67007. ahmadsamir added a comment. Change according to comments REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24171?vs=66899=67007 BRANCH ahmad/foreach-filewidgets (branched from master) REVISION DETAIL

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24171 To: ahmadsamir, kde-frameworks-devel, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir marked 2 inline comments as done. ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kurlnavigator.cpp:803 > Why not a range for here? It should be just fine. I thought button->deleteLater() may change m_navButtons, doesn't it? REPOSITORY R241 KIO REVISION DETAIL

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir updated this revision to Diff 66899. ahmadsamir added a comment. More implicit sharing detach prevention REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24171?vs=66698=66899 BRANCH ahmad/foreach-filewidgets (branched from master) REVISION DETAIL

D24220: [src/ioslaves/*] replace deprecated foreach with range for

ahmadsamir updated this revision to Diff 66895. ahmadsamir added a comment. Use const local var for implicilty shared qstringlist's to prevent detaching REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24220?vs=66815=66895 BRANCH ahmad/foreach-ioslaves2

D24220: [src/ioslaves/*] replace deprecated foreach with range for

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in http.cpp:2612 > need to move that function call into a const local variable Right. A temporary; but since it's a qstringlist it's implicitly shared. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24220

D24220: [src/ioslaves/*] replace deprecated foreach with range for

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN The code compiles and passes all unit tests REPOSITORY R241 KIO BRANCH ahmad/foreach-ioslaves2 (branched from

D24217: [src/ioslaves/*] replace deprecated foreach with range for

ahmadsamir abandoned this revision. ahmadsamir added a comment. git branch mess up on my end, will submit again after cleaning. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24217 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh,

D24217: [src/ioslaves/*] replace deprecated foreach with range for

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN The code compiles and passes all unit tests REPOSITORY R241 KIO BRANCH ahmad/foreach-ioslaves (branched from

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir added a comment. In D24171#536712 , @dfaure wrote: > Cool, I'll review this. BTW don't touch begin/end loops, that's done in D24160 already. Yep, I saw it on the ML :-) REPOSITORY R241

D24171: [src/filewidgets/*] replace deprecated foreach with range for

ahmadsamir created this revision. ahmadsamir added reviewers: kde-frameworks-devel, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN The code compiles and tests pass REPOSITORY R241 KIO BRANCH ahmad/foreach-filewidgets (branched from

D24150: Fix crash on exit in kio_file

ahmadsamir added reviewers: dfaure, broulik. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24150 To: hwti, cfeck, dfaure, broulik Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kcoredirlister.cpp:1044 > Actually, I was wrong. > I talked to the head of the LLVM/clang development team at Apple and he told > me that returning a const value has zero effect whatsoever. I.e. in the > version of your patch

D18915: Fix batchrename changing extension to lower case

ahmadsamir added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18915 To: cfoster, #dolphin, #frameworks, abalaji, dfaure Cc: cfeck, bruns, ngraham, elvisangelaccio, chinmoyr, kde-frameworks-devel, LeGast00n, GB_2, michaelh

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir added a comment. I am sorry, I forgot to use --verbatim, please remove the line about directoriesForCanonicalPath() returning const from the commit message :/ REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23902 To: ahmadsamir, kde-frameworks-devel, dfaure

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir updated this revision to Diff 66393. ahmadsamir marked 2 inline comments as done. ahmadsamir added a comment. Fix patch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23902?vs=66073=66393 BRANCH ahmad/foreach (branched from master) REVISION

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir marked 10 inline comments as done. ahmadsamir added a comment. I think I got all the bits I missed before (sorry about the mess). But I'll sleep on it anyway, will submit in the morning (usually I find mistakes in my code when I look at it again in the morning). Thanks.

D23991: GIT_SILENT fix comment about passwords in debug output

ahmadsamir updated this revision to Diff 66227. ahmadsamir added a comment. Remove redundant comments REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23991?vs=66217=66227 BRANCH ahmad/url-debug-output-passwords (branched from master) REVISION DETAIL

D23991: GIT_SILENT fix comment about passwords in debug output

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY QUrls in debug output always have the password removed (upstream impl.) REPOSITORY R241 KIO BRANCH

D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

ahmadsamir added a comment. Here, in the web-UI, it says authored by dfaure, as it should, but the commit still says authored by me. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23875 To: dfaure, #frameworks, ahmadsamir Cc: dhaumann, kde-frameworks-devel, LeGast00n,

D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

ahmadsamir added a comment. Thanks for the explanation; it filled all the, various, gaps in my reasoning. Looks good to me. (I don't have a dev account, so probably phab.kde.org doesn't show the "accept revision" or similar in its web UI for me). INLINE COMMENTS > dfaure wrote in

D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

ahmadsamir updated this revision to Diff 66104. ahmadsamir edited the summary of this revision. ahmadsamir edited the test plan for this revision. ahmadsamir removed a reviewer: chinmoyr. ahmadsamir added a comment. Refer to one bug report, the others are dupes REPOSITORY R241 KIO CHANGES

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kcoredirlister.cpp:1044 > It's generally considered bad practice to return a const value, because the > caller makes a copy anyway, so this doesn't guarantee anything. And it > removes the benefits of rvalues that can be

D23902: [KCoreDirLister] replace deprecated foreach with range-for

ahmadsamir updated this revision to Diff 66073. ahmadsamir marked 7 inline comments as done. ahmadsamir retitled this revision from "[KCoreDirLister] replace foreach with range-for" to "[KCoreDirLister] replace deprecated foreach with range-for". ahmadsamir added a comment. Fix stuff

D23813: Port away from foreach loops over arguments without calls to owner class

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > Still unsure what you mean with the constness, as the QHash `shortcuts` has > been const all the time, and thus the access methods and its returned > references. If you meant the initially

D23813: Port away from foreach loops over arguments without calls to owner class

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > Is it? > This patch as is now has 3 changes: > > - change a foreach loop over extracted key list of a QHash to then also > access values (2 discouraged things) to iterator-based for loop over the

D23813: Port away from foreach loops over arguments without calls to owner class

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > As someone who looked at a lot of commit history, my recommendation is: don't > do in the same commit. Make it a separate commit with a dedicated commit > message. > While-at-it changes are

D23902: [KCoreDirLister] replace foreach with range-for

ahmadsamir created this revision. ahmadsamir added reviewers: kde-frameworks-devel, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY Also change directoriesForCanonicalPath() to return const TEST PLAN The code compiles and all tests

D23813: Port away from foreach loops over arguments without calls to owner class

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > I try (hard, there are many temptations when looking at all exisiting loop > code) to usually do not too much other improvements but stay on-topic of > commit message change, but will do an

D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

ahmadsamir added a comment. In D23875#529442 , @dhaumann wrote: > Since this crash is in KCoreDirLister, I wonder whether we can find a unit test that reproduces this without the KFileWidget. I think the main issue here is kdirlister

D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY When creating multiple nested new folders, one at a time, in the "save as" dialog, where folders are created and

D23708: [CopyJob] Fix crash when copying an already existing dir and pressing "Skip"

ahmadsamir marked an inline comment as done. ahmadsamir added a comment. Please commit it. Thanks. REPOSITORY R241 KIO BRANCH ahmad/copyjob (branched from master) REVISION DETAIL https://phabricator.kde.org/D23708 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel,

D23708: [CopyJob] Fix crash when copying an already existing dir and pressing "Skip"

ahmadsamir marked an inline comment as done. ahmadsamir added a comment. In D23708#525848 , @dfaure wrote: > Thanks for the fix! > > Do you feel like writing a unittest for it? > JobTest::chmodFileError shows how to use

<    4   5   6   7   8   9   10   >