Merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-15 into lp:ubuntu-filemanager-app

Proposed by Carlos Jose Mazieri on 2015-07-19
Status: Merged
Approved by: Arto Jalkanen on 2015-08-25
Approved revision: 447
Merged at revision: 454
Proposed branch: lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-15
Merge into: lp:ubuntu-filemanager-app
Prerequisite: lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-14
Diff against target: 236 lines (+53/-45)
3 files modified
src/plugin/folderlistmodel/dirmodel.cpp (+27/-16)
src/plugin/folderlistmodel/dirmodel.h (+4/-3)
src/plugin/folderlistmodel/filesystemaction.cpp (+22/-26)
To merge this branch: bzr merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-15
Reviewer Review Type Date Requested Status
Arto Jalkanen 2015-07-19 Approve on 2015-08-25
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-08-25
Review via email: mp+265214@code.launchpad.net

Commit message

Qt QDir object is no longer used in Actions nor in DirModel class, instead inherited LocationItemDir classes are used

Description of the change

Qt QDir object is no longer used in Actions nor in DirModel class, instead inherited LocationItemDir classes are used

To post a comment you must log in.
Arto Jalkanen (ajalkane) wrote :

One diff comment

review: Approve

I was using QScopedPointer where it was really necessary, some new/delete were kept due to performance. I will change both dirmodel.cpp and filesystemaction.cpp to use only QScopedPointer instead of new/delete.

Arto Jalkanen (ajalkane) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/plugin/folderlistmodel/dirmodel.cpp'
2--- src/plugin/folderlistmodel/dirmodel.cpp 2015-07-25 19:25:40 +0000
3+++ src/plugin/folderlistmodel/dirmodel.cpp 2015-08-16 15:16:28 +0000
4@@ -40,6 +40,7 @@
5 #include "disklocation.h"
6 #include "trashlocation.h"
7 #include "netauthenticationdata.h"
8+#include "locationitemdir.h"
9
10
11 #ifndef DO_NOT_USE_TAG_LIB
12@@ -64,6 +65,7 @@
13 #include <QMimeType>
14 #include <QStandardPaths>
15 #include <QList>
16+#include <QScopedPointer>
17
18 #if defined(REGRESSION_TEST_FOLDERLISTMODEL)
19 # include <QColor>
20@@ -642,22 +644,24 @@
21 return retval;
22 }
23
24-void DirModel::mkdir(const QString &newDir)
25+
26+bool DirModel::mkdir(const QString &newDir)
27 {
28- if (!allowAccess(mCurrentDir)) {
29- qDebug() << Q_FUNC_INFO << "Access denied in current path" << mCurrentDir;
30- return;
31- }
32-
33- QDir dir(mCurrentDir);
34- bool retval = dir.mkdir(newDir);
35+ QScopedPointer<LocationItemDir> dir(mCurLocation->newDir(mCurrentDir));
36+ bool retval = dir->mkdir(newDir);
37 if (!retval) {
38 const char *errorStr = strerror(errno);
39 qDebug() << Q_FUNC_INFO << this << "Error creating new directory: " << errno << " (" << errorStr << ")";
40 emit error(QObject::tr("Error creating new folder"), errorStr);
41 } else {
42- onItemAdded(dir.filePath(newDir));
43- }
44+ QScopedPointer<DirItemInfo> subItem(mCurLocation->newItemInfo(newDir));
45+ if (subItem->isRelative())
46+ {
47+ subItem->setFile(mCurrentDir, newDir);
48+ }
49+ onItemAdded(*subItem);
50+ }
51+ return retval;
52 }
53
54 bool DirModel::showDirectories() const
55@@ -728,19 +732,18 @@
56
57 QString DirModel::parentPath() const
58 {
59- QDir dir(mCurrentDir);
60- if (dir.isRoot()) {
61+ const DirItemInfo *dir = mCurLocation->info();
62+ if (dir->isRoot()) {
63 qDebug() << Q_FUNC_INFO << this << "already at root";
64 return mCurrentDir;
65 }
66
67- bool success = dir.cdUp();
68- if (!success) {
69+ if (!canReadDir(dir->absolutePath())) {
70 qWarning() << Q_FUNC_INFO << this << "Failed to to go to parent of " << mCurrentDir;
71 return mCurrentDir;
72 }
73- qDebug() << Q_FUNC_INFO << this << "returning" << dir.absolutePath();
74- return dir.absolutePath();
75+ qDebug() << Q_FUNC_INFO << this << "returning" << dir->absolutePath();
76+ return dir->absolutePath();
77 }
78
79 QString DirModel::homePath() const
80@@ -1259,6 +1262,14 @@
81 return dirFilter;
82 }
83
84+/*!
85+ * \brief DirModel::dirItems() Gets a Dir number of Items, used only for Local Disk
86+ *
87+ * For remote Locations this function is not used
88+ *
89+ * \param fi
90+ * \return A string saying how many items a directory has
91+ */
92 QString DirModel::dirItems(const DirItemInfo& fi) const
93 {
94 int counter = 0;
95
96=== modified file 'src/plugin/folderlistmodel/dirmodel.h'
97--- src/plugin/folderlistmodel/dirmodel.h 2015-07-13 17:29:25 +0000
98+++ src/plugin/folderlistmodel/dirmodel.h 2015-08-16 15:16:28 +0000
99@@ -132,7 +132,7 @@
100 Q_INVOKABLE bool rename(const QString& oldName, const QString& newName);
101 Q_INVOKABLE bool rename(int row, const QString &newName);
102
103- Q_INVOKABLE void mkdir(const QString &newdir);
104+ Q_INVOKABLE bool mkdir(const QString &newdir);
105
106 Q_PROPERTY(bool filterDirectories READ filterDirectories WRITE setFilterDirectories NOTIFY filterDirectoriesChanged)
107 bool filterDirectories() const;
108@@ -436,14 +436,15 @@
109 void needsAuthentication(const QString& user, const QString& urlPath);
110
111 /*!
112- * \brief insertedItem()
113+ * \brief insertedRow()
114 *
115 * It happens when a new file is inserted in an existent view,
116 * for example from \ref mkdir() or \ref paste()
117 *
118 * It can be used to make the new row visible to the user doing a scroll to
119- */
120+ */
121 void insertedRow(int row);
122+
123 /*!
124 * \brief progress()
125 * Sends status about recursive and multi-items remove/move/copy
126
127=== modified file 'src/plugin/folderlistmodel/filesystemaction.cpp'
128--- src/plugin/folderlistmodel/filesystemaction.cpp 2015-08-16 15:16:28 +0000
129+++ src/plugin/folderlistmodel/filesystemaction.cpp 2015-08-16 15:16:28 +0000
130@@ -41,6 +41,7 @@
131 #include "locationsfactory.h"
132 #include "locationitemdiriterator.h"
133 #include "locationitemfile.h"
134+#include "locationitemdir.h"
135
136 #if defined(Q_OS_UNIX)
137 #include <sys/statvfs.h>
138@@ -371,16 +372,15 @@
139 //ActionMove will perform a rename, so no Directory expanding is necessary
140 if (entry->type != ActionMove && info->isDir() && !info->isSymLink())
141 {
142- LocationItemDirIterator *it =
143- action->sourceLocation->newDirIterator(info->absoluteFilePath(),
144- QDir::AllEntries | QDir::System |
145- QDir::NoDotAndDotDot | QDir::Hidden,
146- QDirIterator::Subdirectories);
147+ QScopedPointer<LocationItemDirIterator>
148+ it (action->sourceLocation->newDirIterator(info->absoluteFilePath(),
149+ QDir::AllEntries | QDir::System |
150+ QDir::NoDotAndDotDot | QDir::Hidden,
151+ QDirIterator::Subdirectories));
152 while (it->hasNext() && !it->next().isEmpty())
153 {
154 entry->reversedOrder.prepend(it->fileInfo());
155- }
156- delete it;
157+ }
158 }
159 #if DEBUG_MESSAGES
160 for (int counter = 0; counter < entry->reversedOrder.count(); counter++)
161@@ -651,8 +651,7 @@
162 * \param entry
163 */
164 void FileSystemAction::removeEntry(ActionEntry *entry)
165-{
166- QDir dir;
167+{
168 //do one step at least
169 for(; !m_cancelCurrentAction &&
170 entry->currStep < STEP_FILES &&
171@@ -665,13 +664,13 @@
172 const DirItemInfo &fi = entry->reversedOrder.at(entry->currItem);
173 if (fi.isDir() && !fi.isSymLink())
174 {
175- m_cancelCurrentAction = !dir.rmdir(fi.absoluteFilePath());
176+ QScopedPointer<LocationItemDir> dir(m_curAction->sourceLocation->newDir());
177+ m_cancelCurrentAction = !dir->rmdir(fi.absoluteFilePath());
178 }
179 else
180 {
181- LocationItemFile *qFile = m_curAction->sourceLocation->newFile(fi.absoluteFilePath());
182- m_cancelCurrentAction = !qFile->remove();
183- delete qFile;
184+ QScopedPointer<LocationItemFile> qFile(m_curAction->sourceLocation->newFile(fi.absoluteFilePath()));
185+ m_cancelCurrentAction = !qFile->remove();
186 }
187 #if DEBUG_REMOVE
188 qDebug() << Q_FUNC_INFO << "remove ret=" << !m_cancelCurrentAction << fi.absoluteFilePath();
189@@ -765,16 +764,16 @@
190 )
191 {
192 QString entryDir = targetFrom(entry->reversedOrder.last().absoluteFilePath(), entry);
193- QDir entryDirObj(entryDir);
194- if (!entryDirObj.exists() && entryDirObj.mkpath(entryDir))
195+ QScopedPointer<LocationItemDir> entryDirObj(m_curAction->targetLocation->newDir(entryDir));
196+ if (!entryDirObj->exists() && entryDirObj->mkpath(entryDir))
197 {
198 QScopedPointer <DirItemInfo> item(m_curAction->targetLocation->newItemInfo(entryDir));
199 entry->added = true;
200 notifyActionOnItem(*item, ItemAdded);
201- }
202- }
203- QDir d(path);
204- if (!d.exists() && !d.mkpath(path))
205+ }
206+ }
207+ QScopedPointer<LocationItemDir> d(m_curAction->targetLocation->newDir(path));
208+ if (!d->exists() && !d->mkpath(path))
209 {
210 m_cancelCurrentAction = true;
211 m_errorTitle = QObject::tr("Could not create the directory");
212@@ -794,10 +793,8 @@
213 else
214 if (fi.isDir())
215 {
216- LocationItemFile *qFile = m_curAction->targetLocation->newFile(target);
217- m_cancelCurrentAction = !
218- qFile->setPermissions(fi.permissions());
219- delete qFile;
220+ QScopedPointer<LocationItemFile> qFile(m_curAction->targetLocation->newFile(target));
221+ m_cancelCurrentAction = !qFile->setPermissions(fi.permissions());
222 if (m_cancelCurrentAction)
223 {
224 m_errorTitle = QObject::tr("Could not set permissions to dir");
225@@ -1393,9 +1390,8 @@
226 #if defined(DEBUG_MESSAGES) || defined(REGRESSION_TEST_FOLDERLISTMODEL)
227 qDebug() << Q_FUNC_INFO << dir << "being moved to" << tempDir;
228 #endif
229- LocationItemFile *qFile = m_curAction->targetLocation->newFile(dir);
230- bool removed = qFile->rename(tempDir);
231- delete qFile;
232+ QScopedPointer<LocationItemFile> qFile(m_curAction->targetLocation->newFile(dir));
233+ bool removed = qFile->rename(tempDir);
234 if (removed)
235 {
236 if (m_curAction->auxAction == 0)

Subscribers

People subscribed via source and target branches