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

Proposed by Carlos Jose Mazieri on 2015-07-18
Status: Merged
Approved by: Arto Jalkanen on 2015-08-06
Approved revision: 433
Merged at revision: 438
Proposed branch: lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-03
Merge into: lp:ubuntu-filemanager-app
Prerequisite: lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-02
Diff against target: 135 lines (+19/-52)
2 files modified
src/plugin/folderlistmodel/dirmodel.cpp (+17/-48)
src/plugin/folderlistmodel/dirmodel.h (+2/-4)
To merge this branch: bzr merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-03
Reviewer Review Type Date Requested Status
Arto Jalkanen 2015-07-18 Approve on 2015-07-24
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-07-18
Review via email: mp+265194@code.launchpad.net

Commit message

Removed some QFileInfo dependency because it will not work for remote locations, using class Location instead.

Description of the change

Replaced some disk access using Qt Objects by Location similar objects

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

Several places that this comment applies to:

Why don't you use d(setParentIfRelative(folderName)); here also?

My understanding of C++ is that is more efficient than using assignment to created object. With assignment the object is first created, and then it's operator=() called to set instance's variables. If using directly copy constructor, the compiler can optimize the creation of the object.

I'll approve this nevertheless as it's a minor thing and you can consider yourself whether to change it or top-approve this change.

review: Approve

Thanks for looking at this, for sure I agree, I just was not paying attention on this.

I will change that in the samba-actions-04 as it changes again DirModel::canReadDir(), I think that is possible to generate a merge conflict changing it here.

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-18 21:32:12 +0000
3+++ src/plugin/folderlistmodel/dirmodel.cpp 2015-07-18 21:32:12 +0000
4@@ -1509,80 +1509,51 @@
5
6 bool DirModel::existsDir(const QString &folderName) const
7 {
8- DirItemInfo d(setParentIfRelative(folderName));
9+ DirItemInfo d = setParentIfRelative(folderName);
10 return d.exists() && d.isDir();
11 }
12
13 bool DirModel::canReadDir(const QString &folderName) const
14 {
15- DirItemInfo d(setParentIfRelative(folderName));
16- return canReadDir(d.diskFileInfo());
17+ DirItemInfo d = setParentIfRelative(folderName);
18+ return d.isDir() && d.isReadable();
19 }
20
21-bool DirModel::canReadDir(const QFileInfo & d) const
22-{
23- return d.exists() && d.isDir() && d.isReadable() && d.isExecutable();
24-}
25
26 bool DirModel::existsFile(const QString &fileName) const
27 {
28- DirItemInfo f(setParentIfRelative(fileName));
29+ DirItemInfo f = setParentIfRelative(fileName);
30 return f.exists() && f.isFile();
31 }
32
33 bool DirModel::canReadFile(const QString &fileName) const
34 {
35- DirItemInfo f(setParentIfRelative(fileName));
36- return canReadFile(f.diskFileInfo());
37-}
38-
39-bool DirModel::canReadFile(const QFileInfo &f) const
40-{
41- return f.exists() && f.isFile() && f.isReadable();
42-}
43-
44+ DirItemInfo f = setParentIfRelative(fileName);
45+ return f.isReadable() && f.isFile();
46+}
47
48
49 QDateTime DirModel::curPathCreatedDate() const
50-{
51- QDateTime d;
52- QFileInfo f(mCurrentDir);
53- if (f.exists())
54- {
55- d = f.created();
56- }
57- return d;
58+{
59+ return mCurLocation->currentInfo()->created();
60 }
61
62
63 QDateTime DirModel::curPathModifiedDate() const
64 {
65- QDateTime d;
66- QFileInfo f(mCurrentDir);
67- if (f.exists())
68- {
69- d = f.lastModified();
70- }
71- return d;
72+ return mCurLocation->currentInfo()->lastModified();
73 }
74
75
76 QDateTime DirModel::curPathAccessedDate() const
77 {
78- QDateTime d;
79- QFileInfo f(mCurrentDir);
80- if (f.exists())
81- {
82- d = f.lastRead();
83- }
84- return d;
85+ return mCurLocation->currentInfo()->lastRead();
86 }
87
88
89 bool DirModel::curPathIsWritable() const
90 {
91- QFileInfo f(mCurrentDir);
92- return f.exists() && f.isWritable();
93+ return mCurLocation->currentInfo()->isWritable();
94 }
95
96 QString DirModel::curPathCreatedDateLocaleShort() const
97@@ -1621,16 +1592,14 @@
98 }
99
100
101-QFileInfo DirModel::setParentIfRelative(const QString &fileOrDir) const
102+DirItemInfo DirModel::setParentIfRelative(const QString &fileOrDir) const
103 {
104- QFileInfo myFi(fileOrDir);
105- if (myFi.isRelative())
106+ QScopedPointer<DirItemInfo> myFi(mCurLocation->newItemInfo(fileOrDir));
107+ if (!myFi->isAbsolute())
108 {
109- myFi.setFile(mCurrentDir, fileOrDir);
110- QFileInfo abs(myFi.absoluteFilePath());
111- myFi = abs;
112+ myFi->setFile(mCurrentDir, fileOrDir);
113 }
114- return myFi;
115+ return *myFi;
116 }
117
118
119
120=== modified file 'src/plugin/folderlistmodel/dirmodel.h'
121--- src/plugin/folderlistmodel/dirmodel.h 2015-03-14 19:07:13 +0000
122+++ src/plugin/folderlistmodel/dirmodel.h 2015-07-18 21:32:12 +0000
123@@ -475,10 +475,8 @@
124 QDir::Filter currentDirFilter() const;
125 QString dirItems(const DirItemInfo& fi) const;
126 bool cdIntoItem(const DirItemInfo& fi);
127- bool openItem(const DirItemInfo& fi);
128- bool canReadDir(const QFileInfo& d) const;
129- bool canReadFile(const QFileInfo& f) const;
130- QFileInfo setParentIfRelative(const QString &fileOrDir) const;
131+ bool openItem(const DirItemInfo& fi);
132+ DirItemInfo setParentIfRelative(const QString &fileOrDir) const;
133 void setPathFromCurrentLocation();
134
135 private:

Subscribers

People subscribed via source and target branches