Merge lp:~stolowski/unity-scopes-api/fix-1393382-rtm into lp:unity-scopes-api/rtm-14.09

Proposed by Paweł Stołowski
Status: Merged
Approved by: Michi Henning
Approved revision: 249
Merged at revision: 259
Proposed branch: lp:~stolowski/unity-scopes-api/fix-1393382-rtm
Merge into: lp:unity-scopes-api/rtm-14.09
Diff against target: 38 lines (+17/-9)
1 file modified
scoperegistry/FindFiles.cpp (+17/-9)
To merge this branch: bzr merge lp:~stolowski/unity-scopes-api/fix-1393382-rtm
Reviewer Review Type Date Requested Status
Michi Henning (community) Approve
Review via email: mp+242067@code.launchpad.net

Commit message

Catch file exception while iterating over individual scope dirs, so that a single failure doesn't break all other scopes.

Description of the change

Catch file exception while iterating over individual scope dirs, so that a single failure doesn't break all other scopes.

To post a comment you must log in.
Revision history for this message
Michi Henning (michihenning) wrote :

Looks good, thanks!

Very minor fix:

error("could not open scope directory: " + scope_dir + "\n");

The trailing newline shouldn't be there. error() automatically appends a newline. (You need a newline only if you want to force an error message to include a line break explictly.)

review: Needs Fixing
249. By Paweł Stołowski on 2014-11-19

No need for newline.

Revision history for this message
Michi Henning (michihenning) wrote :

Thanks!

review: Approve
250. By Paweł Stołowski on 2015-01-12

Merged rtm.

251. By Paweł Stołowski on 2015-01-12

Catch ResourceException, not FileException.

Revision history for this message
Michi Henning (michihenning) wrote :

Right, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scoperegistry/FindFiles.cpp'
2--- scoperegistry/FindFiles.cpp 2014-08-28 00:20:56 +0000
3+++ scoperegistry/FindFiles.cpp 2015-01-12 17:59:13 +0000
4@@ -116,18 +116,26 @@
5 auto scope_dirs = find_entries(install_dir, Directory);
6 for (auto scope_dir : scope_dirs)
7 {
8- auto configs = find_scope_dir_configs(scope_dir, suffix);
9- for (auto config : configs)
10+ try
11 {
12- auto const it = scopes_seen.find(config.first);
13- if (it != scopes_seen.end())
14+ auto configs = find_scope_dir_configs(scope_dir, suffix);
15+ for (auto config : configs)
16 {
17- error("ignoring second instance of non-unique scope: " + config.second + "\n"
18- "previous instance: " + it->second);
19- continue;
20+ auto const it = scopes_seen.find(config.first);
21+ if (it != scopes_seen.end())
22+ {
23+ error("ignoring second instance of non-unique scope: " + config.second + "\n"
24+ "previous instance: " + it->second);
25+ continue;
26+ }
27+ scopes_seen[config.first] = config.second;
28+ files.insert(config);
29 }
30- scopes_seen[config.first] = config.second;
31- files.insert(config);
32+ }
33+ catch (ResourceException const& e)
34+ {
35+ error(e.what());
36+ error("could not open scope directory: " + scope_dir);
37 }
38 }
39

Subscribers

People subscribed via source and target branches

to all changes: