Merge lp://staging/~submarine/ubuntu-scopes/sshsearch-new-api into lp://staging/~submarine/ubuntu-scopes/sshsearch
Proposed by
Mark Tully
Status: | Needs review |
---|---|
Proposed branch: | lp://staging/~submarine/ubuntu-scopes/sshsearch-new-api |
Merge into: | lp://staging/~submarine/ubuntu-scopes/sshsearch |
Diff against target: |
508 lines (+287/-189) 2 files modified
src/unity_sshsearch_daemon.py (+230/-152) tests/test_sshsearch.py (+57/-37) |
To merge this branch: | bzr merge lp://staging/~submarine/ubuntu-scopes/sshsearch-new-api |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Henstridge | Needs Fixing | ||
Review via email: mp+155078@code.staging.launchpad.net |
Description of the change
Updating to Unity 7 api
To post a comment you must log in.
Unmerged revisions
- 5. By Mark Tully
-
Ported to Unity 7 API
Updated tests
This scope is already working with Libunity 7, right? The diff shows it using Unity.Deprecate dScope, which while it isn't the new recommended new API it will work.
From the look of it, the two file monitors set up in the search() routine don't do anything. For example, the new read_config() function appears to have no side effects, so the following code looks like it won't do anything useful:
+ config_file = Gio.file_ new_for_ path(SSHCONFIG_ EXPAND) file.monitor_ file(flags= Gio.FileMonitor Flags.NONE, cancellable=None) monitor. connect( 'changed' , read_config)
+ config_monitor = config_
+ config_
The original code doesn't look much better, setting up the file monitors and rereading config files every time update_ results_ model is called.
If the intent is to only parse the files when they change, I'd expect the scope to set up the file monitors once on start up, and use the cached config data to respond to searches.
Also, constructs like the following are difficult to read:
+ if not 'uri' in i or not i['uri'] or i['uri'] == '':
The following should provide the same result:
if not i.get('uri'):
With the if statements shortened, you should have space to use a more meaningful identifier than 'i' too.