From kde-core-devel Mon Jul 21 18:22:45 2014 From: =?utf-8?q?Thomas_L=C3=BCbking?= Date: Mon, 21 Jul 2014 18:22:45 +0000 To: kde-core-devel Subject: Re: Review Request 119372: Fix Bug 337486 - Should not permit moving of read-only folder to a differ Message-Id: <20140721182245.14616.430 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=140596699428910 --===============8833514438715228107== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119372/#review62821 ----------------------------------------------------------- lib/konq/konq_operations.cpp this indention is wrong (ensure to use the present indention, usually 4 spaces per level and NO tabs) lib/konq/konq_operations.cpp there are two whitespaces between "if" and "(" - not that the surrounding lines would be very white space consistent, but nevertheless ;-) lib/konq/konq_operations.cpp Try to twist this: if (!local) ... else if (QFileInfo((*it).directory()).isWritable()) { QFileInfo pathInfo(QFileInfo((*it).path()); ... } This allows you to re-use the QFileInfo object w/o useless overhead and also removes a branch level. - Thomas Lübking On Juli 21, 2014, 2:12 nachm., Arjun Ak wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119372/ > ----------------------------------------------------------- > > (Updated Juli 21, 2014, 2:12 nachm.) > > > Review request for KDE Base Apps. > > > Bugs: 337486 > http://bugs.kde.org/show_bug.cgi?id=337486 > > > Repository: kde-baseapps > > > Description > ------- > > Fixes Bug 337486 > > > Diffs > ----- > > lib/konq/konq_operations.cpp 220a90a > > Diff: https://git.reviewboard.kde.org/r/119372/diff/ > > > Testing > ------- > > > Thanks, > > Arjun Ak > > --===============8833514438715228107== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119372/

lib/konq/konq_operations.cpp (Diff revision 3)
void KonqOperations::doDropFileCopy()
474	mlst.append(*it);	474	if (local) {

this indention is wrong (ensure to use the present indention, usually 4 spaces per level and NO tabs)

lib/konq/konq_operations.cpp (Diff revision 3)
void KonqOperations::doDropFileCopy()
		475	if (QFileInfo((it).directory()).isWritable() && (!QFileInfo((it).path()).isDir() \|\|

there are two whitespaces between "if" and "(" - not that the surrounding lines would be very white space consistent, but nevertheless ;-)

lib/konq/konq_operations.cpp (Diff revision 3)
void KonqOperations::doDropFileCopy()
		479	} else {

Try to twist this:
if (!local)

    ...

else if (QFileInfo((it).directory()).isWritable()) {

    QFileInfo pathInfo(QFileInfo((it).path());

    ...

}
This allows you to re-use the QFileInfo object w/o useless overhead and also removes a branch level.

- Thomas Lübking

On Juli 21st, 2014, 2:12 nachm. UTC, Arjun Ak wrote:

Review request for KDE Base Apps.

By Arjun Ak.

Updated Juli 21, 2014, 2:12 nachm.

Bugs: 337486

Repository: kde-baseapps

Description

Fixes Bug 337486

Diffs

lib/konq/konq_operations.cpp (220a90a)

--===============8833514438715228107==--