Merge lp:~evfool/software-center/lp752376 into lp:software-center

Proposed by Robert Roth on 2012-06-03
Status: Merged
Merged at revision: 3025
Proposed branch: lp:~evfool/software-center/lp752376
Merge into: lp:software-center
Diff against target: 78 lines (+43/-0)
2 files modified
softwarecenter/ui/gtk3/app.py (+7/-0)
test/gtk3/test_app.py (+36/-0)
To merge this branch: bzr merge lp:~evfool/software-center/lp752376
Reviewer Review Type Date Requested Status
software-store-developers 2012-06-03 Pending
Review via email: mp+108481@code.launchpad.net

Description of the change

Updated the code to also work with file:// style deb filenames, as requested in LP #752376, also added a unittest to check if the DebFileApplication is loaded after startup.

To post a comment you must log in.
Michael Vogt (mvo) wrote :

Thanks a lot for this branch, its really great to see all your work around software-center!

It looks great there are two small issues:
- the file:// should probably start with file:/ just to ensure that the widest possible range if covered, I think some file managers pass file:/, file:// or even file:///
- the regexp is ideally updated as well for this as the prefix already includes the // so the regexp is not matching exactly what we need here. we can probably simplify it quite a bit and just use "os.path.normpath" after removing the prefix "file:/" to cover prefixed "/".

It would be great if you could have a look, but I'm happy to work on this later today too if you are busy.

lp:~evfool/software-center/lp752376 updated on 2012-06-04
3026. By Robert Roth on 2012-06-04

Fixed to work with multiple path separators by normalizing the file path

3027. By Robert Roth on 2012-06-04

Removed unnecessary file prefix regex

Robert Roth (evfool) wrote :

CHanged the logic to strip the file: and normalize the remaining part as a path.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/ui/gtk3/app.py'
2--- softwarecenter/ui/gtk3/app.py 2012-06-01 17:51:05 +0000
3+++ softwarecenter/ui/gtk3/app.py 2012-06-04 11:09:18 +0000
4@@ -39,6 +39,7 @@
5
6 import webbrowser
7
8+from os.path import normpath
9 from gettext import gettext as _
10
11 # purely to initialize the netstatus
12@@ -126,6 +127,7 @@
13
14 LOG = logging.getLogger(__name__)
15 PACKAGE_PREFIX = 'apt:'
16+FILE_PREFIX = 'file:'
17 # "apt:///" is a valid prefix for 'apt:pkgname' in alt+F2 in gnome
18 PACKAGE_PREFIX_REGEX = re.compile('^%s(?:/{2,3})*' % PACKAGE_PREFIX)
19 SEARCH_PREFIX = 'search:'
20@@ -163,6 +165,11 @@
21 # remove the initial search prefix
22 items[0] = items[0].replace(SEARCH_PREFIX, '', 1)
23 search_text = SearchSeparators.REGULAR.join(items)
24+ elif items[0].startswith(FILE_PREFIX):
25+ # strip away the initial file: prefix, if present
26+ items[0] = items[0].replace(FILE_PREFIX, '', 1)
27+ # normalize the path to strip duplicate path separators, etc
28+ items[0] = normpath(items[0])
29 else:
30 # strip away the initial apt: prefix, if present
31 items[0] = re.sub(PACKAGE_PREFIX_REGEX, '', items[0])
32
33=== modified file 'test/gtk3/test_app.py'
34--- test/gtk3/test_app.py 2012-05-21 20:10:39 +0000
35+++ test/gtk3/test_app.py 2012-06-04 11:09:18 +0000
36@@ -70,6 +70,42 @@
37 search_text, result_app = app.parse_packages_args(fname)
38 self.assertIsInstance(result_app, DebFileApplication)
39
40+ def test_item_with_file_prefix_one_slash(self):
41+ """ Pass an existing filename with a file:/ prefix. """
42+ # pass a real deb here
43+ fname = os.path.join(os.path.dirname(os.path.abspath(__file__)),
44+ "..", "data", "test_debs", "gdebi-test1.deb")
45+ assert os.path.exists(fname)
46+ fname = "file:/" + fname
47+ # test once as string and as list
48+ for items in ( fname, [fname] ):
49+ search_text, result_app = app.parse_packages_args(fname)
50+ self.assertIsInstance(result_app, DebFileApplication)
51+
52+ def test_item_with_file_prefix_two_slashes(self):
53+ """ Pass an existing filename with a file:// prefix. """
54+ # pass a real deb here
55+ fname = os.path.join(os.path.dirname(os.path.abspath(__file__)),
56+ "..", "data", "test_debs", "gdebi-test1.deb")
57+ assert os.path.exists(fname)
58+ fname = "file://" + fname
59+ # test once as string and as list
60+ for items in ( fname, [fname] ):
61+ search_text, result_app = app.parse_packages_args(fname)
62+ self.assertIsInstance(result_app, DebFileApplication)
63+
64+ def test_item_with_file_prefix_three_slashes(self):
65+ """ Pass an existing filename with a file:/// prefix. """
66+ # pass a real deb here
67+ fname = os.path.join(os.path.dirname(os.path.abspath(__file__)),
68+ "..", "data", "test_debs", "gdebi-test1.deb")
69+ assert os.path.exists(fname)
70+ fname = "file:///" + fname
71+ # test once as string and as list
72+ for items in ( fname, [fname] ):
73+ search_text, result_app = app.parse_packages_args(fname)
74+ self.assertIsInstance(result_app, DebFileApplication)
75+
76 def test_item_is_invalid_file(self):
77 """ Pass an invalid file item """
78 fname = __file__