Merge lp:~marcustomlinson/unity-scopes-shell/lp-1583055 into lp:unity-scopes-shell

Proposed by Marcus Tomlinson on 2016-05-19
Status: Merged
Approved by: Paweł Stołowski on 2016-05-19
Approved revision: 317
Merged at revision: 323
Proposed branch: lp:~marcustomlinson/unity-scopes-shell/lp-1583055
Merge into: lp:unity-scopes-shell
Diff against target: 81 lines (+13/-10)
2 files modified
src/Unity/settingsmodel.cpp (+12/-9)
src/Unity/settingsmodel.h (+1/-1)
To merge this branch: bzr merge lp:~marcustomlinson/unity-scopes-shell/lp-1583055
Reviewer Review Type Date Requested Status
Paweł Stołowski 2016-05-19 Approve on 2016-05-19
PS Jenkins bot continuous-integration Pending
Review via email: mp+295161@code.launchpad.net

Commit message

Only create an empty settings file when attempting to write to one

To post a comment you must log in.
Marcus Tomlinson (marcustomlinson) wrote :

I have tested this on the phone. If you'd like to confirm, here's a build: https://code.launchpad.net/~unity-api-team/+archive/ubuntu/dev-build-4

Paweł Stołowski (stolowski) wrote :

Looks good, thanks Markus!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Unity/settingsmodel.cpp'
2--- src/Unity/settingsmodel.cpp 2016-04-19 08:04:55 +0000
3+++ src/Unity/settingsmodel.cpp 2016-05-19 07:18:35 +0000
4@@ -86,12 +86,11 @@
5
6 try
7 {
8- tryLoadSettings();
9+ tryLoadSettings(true);
10 }
11- catch(const unity::FileException& e)
12+ catch(const unity::FileException&)
13 {
14- // Something has gone wrong, at this point we'll just have to continue with a null m_settings.
15- qWarning() << "SettingsModel::SettingsModel: Failed to read settings file:" << e.what();
16+ // No settings file found, at this point we'll just have to continue with a null m_settings.
17 }
18
19 for (const auto &it : settingsDefinitions.toList())
20@@ -179,7 +178,7 @@
21 {
22 try
23 {
24- tryLoadSettings();
25+ tryLoadSettings(true);
26 switch (data->variantType)
27 {
28 case QVariant::Bool:
29@@ -269,7 +268,7 @@
30 QVariant result;
31 try
32 {
33- tryLoadSettings();
34+ tryLoadSettings(true);
35 switch (data->variantType)
36 {
37 case QVariant::Bool:
38@@ -486,7 +485,7 @@
39 {
40 try
41 {
42- tryLoadSettings();
43+ tryLoadSettings(false);
44 switch (value.type())
45 {
46 case QVariant::Bool:
47@@ -526,15 +525,19 @@
48 }
49 }
50
51-void SettingsModel::tryLoadSettings() const
52+void SettingsModel::tryLoadSettings(bool read_only) const
53 {
54 if (!m_settings)
55 {
56 QFileInfo checkFile(m_settings_path);
57 if (!checkFile.exists() || !checkFile.isFile())
58 {
59+ if (read_only)
60+ {
61+ throw unity::FileException("Could not locate a settings file at: " + m_settings_path.toStdString(), -1);
62+ }
63 // Config file does not exist, so we create an empty one.
64- if (!QFile(m_settings_path).open(QFile::WriteOnly))
65+ else if (!QFile(m_settings_path).open(QFile::WriteOnly))
66 {
67 throw unity::FileException("Could not create an empty settings file at: " + m_settings_path.toStdString(), -1);
68 }
69
70=== modified file 'src/Unity/settingsmodel.h'
71--- src/Unity/settingsmodel.h 2016-04-18 16:18:20 +0000
72+++ src/Unity/settingsmodel.h 2016-05-19 07:18:35 +0000
73@@ -91,7 +91,7 @@
74 void settings_timeout();
75
76 private:
77- void tryLoadSettings() const;
78+ void tryLoadSettings(bool read_only) const;
79
80 protected:
81 mutable QMutex m_mutex;

Subscribers

People subscribed via source and target branches

to all changes: