Merge lp:~gary-lasker/software-center/title-summary-fixes into lp:software-center

Proposed by Gary Lasker
Status: Merged
Merged at revision: 1172
Proposed branch: lp:~gary-lasker/software-center/title-summary-fixes
Merge into: lp:software-center
Diff against target: 263 lines (+80/-45)
7 files modified
debian/changelog (+14/-3)
softwarecenter/db/application.py (+47/-24)
softwarecenter/db/database.py (+5/-8)
softwarecenter/view/appview.py (+2/-7)
softwarecenter/view/availablepane.py (+5/-1)
softwarecenter/view/installedpane.py (+5/-1)
softwarecenter/view/softwarepane.py (+2/-1)
To merge this branch: bzr merge lp:~gary-lasker/software-center/title-summary-fixes
Reviewer Review Type Date Requested Status
software-store-developers Pending
Review via email: mp+35242@code.launchpad.net

Description of the change

This branch is to fix bug 636004.

Previously, there were two places that implemented the "special case" code that substituted summary for title and pkgname for summary (as required by the spec). The first was in applist.py for the applist view (where, as mpt noted, the substitution was working fine while the details view was broken). The second was in the db.AppDetails object, and this was used by the app details view.

Since we need to maintain a lightweight object for use with applist, I consolidated the code into a pair of calls (get_display_name and get_display_summary) in the lightweight db.Application object itself. By pulling these out of db.AppDetails we avoid having to generate an instance of db.AppDetails (a relatively heavyweight object) for every item in the applist. I kept the db.AppDetails API unchanged, however, and just reimplemented the display_name and display_summary properties to call out to the db.Application object's methods.

Finally, I wired the navigation button to use the display name rather than the simple pkgname as it was using up until now, so that bug is fixed as well. Now, the applist, the appdetails and the navigation bar all share the same code.

I'm sure there are further opportunities for optimization (for instance, once we have appdetails generated for the navigation bar, we could pass it for use in appdetails), but I stopped here as this seemed the minimum surgery to do the trick.

Thanks!

To post a comment you must log in.
Gary Lasker (gary-lasker) wrote :

Bah, I'm seeing a freeze when trying to view the app details for the "A Better CD Encoder" package (see bug 636843). This doesn't seem related to my branch as I also see it happening in 2.1.18.1.

Anyway, since this is the test package for this bug, I wanted to report it here as well. You can work around the problem (if you see it) by first doing a search to narrow the length of the list before clicking "More Info" (just type "abcd" into the search bar).

1168. By Gary Lasker on 2010-09-13

merge lp:~kiwinote/software-center/title-summary-fixes, many thanks kiwinote

Gary Lasker (gary-lasker) wrote :

I just merged some additional fixes from kiwinote. Many thanks!

Here is a summary from kiwinote of his changes:
---
We need to replace the 'if self.appname' with 'if db.get_appname(doc)' as when we init s-c with a command line argument (eg software-center gdebi), we have self.appname = None. (Or we could look at actually setting self.appname in this case.)

We add 'if doc' incase the doc=None (ie deb files. For deb file summaries this isn't crucial, as we override this anyway in a new function). We also need to add 'return self.name' as a fallback for the appdetailsview for deb files.

Display correct title in nav bar for apturls/deb files. (not really the most ideal of moves, as we open the deb files twice, but it seems acceptable (and encouragable) to do for maverick. (Given that a very little bit of extra time on a slowish startup doesn't make that much difference ;) ) )

(I also merged a revision from one of my other branches so that I could test the corner case deb files as well.)

1169. By Gary Lasker on 2010-09-14

merge with trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2010-09-13 16:27:49 +0000
3+++ debian/changelog 2010-09-14 02:59:44 +0000
4@@ -6,6 +6,16 @@
5 * softwarecenter/backend/channel.py:
6 - always display the partner channel, even if its
7 source is not enabled (LP: #635003)
8+ * softwarecenter/db/application.py,
9+ softwarecenter/db/database.py,
10+ softwarecenter/view/appview.py,
11+ softwarecenter/view/softwarepane.py:
12+ - make StoreDatabase.get_appname return None if the
13+ name is not defined in the xapian doc
14+ - consolidate code to generate application name and
15+ summary for display in the UI to a single place
16+ - wire applist, appdetails and the navigation bar
17+ to use it (LP: #636004)
18
19 [ Kiwinote ]
20 * data/software-center.menu.in:
21@@ -14,9 +24,12 @@
22 - allow 'for purchase' to have an icon in non-English locales (LP: #636242)
23 * softwarecenter/db/application/py:
24 - try to open deb file, except on any sort of error (LP: #633379)
25+ - reload pkg cache object and xapian doc if it turns out that the pkgname
26+ of a deb file is different than what we guessed on basis of the file name
27 * softwarecenter/view/appdetailsview_gtk.py:
28 - use the display name for app entry in 'where is it' (LP: #635464)
29 - restore correct padding in addons status bar (LP: #625230)
30+ * minor tweaks to lp:~gary-lasker/software-center/title-summary-fixes
31
32 [ Matthew Paul Thomas ]
33 * data/software-center.menu.in:
34@@ -26,7 +39,7 @@
35 [ Colin Watson ]
36 * Fix update-apt-xapian-index crashes when the Dir::Cache::pkgcache file
37 doesn't exist (LP: #267330).
38-
39+
40 [ Michael Vogt ]
41 * softwarecenter/backend/aptd.py:
42 - fix "reload()" in enable_channel() and only update the particular
43@@ -36,8 +49,6 @@
44 * merged lp:~mmcg069/software-center/Bug635375
45 * merged lp:~mmcg069/software-center/tiny-pathbar-tweak
46
47- -- Michael Vogt <michael.vogt@ubuntu.com> Mon, 13 Sep 2010 16:18:45 +0200
48-
49 software-center (2.1.18.1) maverick; urgency=low
50
51 [ Colin Watson ]
52
53=== modified file 'softwarecenter/db/application.py'
54--- softwarecenter/db/application.py 2010-09-11 14:49:38 +0000
55+++ softwarecenter/db/application.py 2010-09-14 02:59:44 +0000
56@@ -69,6 +69,28 @@
57 def get_details(self, db):
58 """ return a new AppDetails object for this application """
59 return AppDetails(db, application=self)
60+ def get_display_name(self, db, doc):
61+ """ Return the application name as it should be displayed in the UI
62+ If the appname is defined, just return it, else return
63+ the summary (per the spec)
64+ """
65+ if doc:
66+ appname = db.get_appname(doc)
67+ if appname:
68+ return appname
69+ else:
70+ return db.get_summary(doc)
71+ def get_display_summary(self, db, doc):
72+ """ Return the application summary as it should be displayed in the UI
73+ If the appname is defined, return the application summary, else return
74+ the application's pkgname (per the spec)
75+ """
76+ if doc:
77+ if db.get_appname(doc):
78+ return db.get_summary(doc)
79+ else:
80+ return db.get_pkgname(doc)
81+
82 # special methods
83 def __hash__(self):
84 return ("%s:%s" % (self.appname, self.pkgname)).__hash__()
85@@ -96,7 +118,7 @@
86 if not debfile.endswith(".deb") and not debfile.count('/') >= 2:
87 raise ValueError("Need a deb file, got '%s'" % debfile)
88 debname = os.path.splitext(os.path.basename(debfile))[0]
89- self.appname = debname.split('_')[0].capitalize()
90+ self.appname = ""
91 self.pkgname = debname.split('_')[0].lower()
92 self.request = debfile
93 def get_details(self, db):
94@@ -295,38 +317,24 @@
95
96 @property
97 def display_name(self):
98- """ Return the name as it should be displayed in the UI
99-
100- Note that this may not corespond to the Appname as the
101- spec says the name should be the summary for packages
102- and the summary the pkgname
103+ """ Return the application name as it should be displayed in the UI
104+ If the appname is defined, just return it, else return
105+ the summary (per the spec)
106 """
107 if self._error_not_found:
108 return self._error
109 if self._doc:
110- name = self._db.get_appname(self._doc)
111- if name:
112- return name
113- else:
114- # by spec..
115- return self._db.get_summary(self._doc)
116+ return self._app.get_display_name(self._db, self._doc)
117 return self.name
118
119 @property
120 def display_summary(self):
121- """ Return the summary as it should be displayed in the UI
122-
123- Note that this may not corespond to the summary value as the
124- spec says the name should be the summary for packages
125- and the summary the pkgname
126+ """ Return the application summary as it should be displayed in the UI
127+ If the appname is defined, return the application summary, else return
128+ the application's pkgname (per the spec)
129 """
130 if self._doc:
131- name = self._db.get_appname(self._doc)
132- if name:
133- return self._db.get_summary(self._doc)
134- else:
135- # by spec..
136- return self._db.get_pkgname(self._doc)
137+ return self._app.get_display_summary(self._db, self._doc)
138 return ""
139
140 @property
141@@ -592,9 +600,24 @@
142 self._error_not_found = _("The file \"%s\" could not be opened.") % self._app.request
143 return
144
145- if self.pkgname:
146+ if self.pkgname and self.pkgname != self._app.pkgname:
147+ # this happens when the deb file has a quirky file name
148 self._app.pkgname = self.pkgname
149
150+ # load pkg cache
151+ self._pkg = None
152+ if (self._app.pkgname in self._cache and
153+ self._cache[self._app.pkgname].candidate):
154+ self._pkg = self._cache[self._app.pkgname]
155+
156+ # load xapian document
157+ self._doc = None
158+ try:
159+ self._doc = self._db.get_xapian_document(
160+ self._app.appname, self._app.pkgname)
161+ except:
162+ pass
163+
164 # check deb and set failure state on error
165 if not self._deb.check():
166 self._error = self._deb._failure_string
167
168=== modified file 'softwarecenter/db/database.py'
169--- softwarecenter/db/database.py 2010-09-10 09:17:07 +0000
170+++ softwarecenter/db/database.py 2010-09-14 02:59:44 +0000
171@@ -223,19 +223,16 @@
172 """ Return a packagename from a xapian document """
173 pkgname = doc.get_value(XAPIAN_VALUE_PKGNAME)
174 # if there is no value it means we use the apt-xapian-index
175- # that store the pkgname in the data field directly
176+ # that stores the pkgname in the data field directly
177 if not pkgname:
178 pkgname = doc.get_data()
179 return pkgname
180
181 def get_appname(self, doc):
182- """ Return a appname from a xapian document """
183- appname = doc.get_value(XAPIAN_VALUE_APPNAME)
184- # if there is no value it means we use the apt-xapian-index
185- # and that has no appname
186- if not appname:
187- appname = doc.get_data()
188- return appname
189+ """ Return a appname from a xapian document, or None if
190+ a value for appname cannot be found in the document
191+ """
192+ return doc.get_value(XAPIAN_VALUE_APPNAME)
193
194 def get_iconname(self, doc):
195 """ Return the iconname from the xapian document """
196
197=== modified file 'softwarecenter/view/appview.py'
198--- softwarecenter/view/appview.py 2010-09-10 09:17:07 +0000
199+++ softwarecenter/view/appview.py 2010-09-14 02:59:44 +0000
200@@ -540,13 +540,8 @@
201 summary = self.db.get_summary(doc)
202 return "%s\n%s" % (appname, summary)
203 elif column == self.COL_MARKUP:
204- appname = app.appname
205- summary = self.db.get_summary(doc)
206- # SPECIAL CASE: the spec says that when there is no appname,
207- # the summary should be displayed as appname
208- if not appname:
209- appname = summary
210- summary = app.pkgname
211+ appname = app.get_display_name(self.db, doc)
212+ summary = app.get_display_summary(self.db, doc)
213 if self.db.is_appname_duplicated(appname):
214 appname = "%s (%s)" % (appname, app.pkgname)
215 s = "%s\n<small>%s</small>" % (
216
217=== modified file 'softwarecenter/view/availablepane.py'
218--- softwarecenter/view/availablepane.py 2010-09-13 09:14:31 +0000
219+++ softwarecenter/view/availablepane.py 2010-09-14 02:59:44 +0000
220@@ -486,7 +486,11 @@
221 else:
222 self.apps_category = Category("deb", "deb", None, None, False, True, None)
223 self.current_app_by_category[self.apps_category] = app
224- self.navigation_bar.add_with_id(app.name, self.on_navigation_details, "details", animate=True)
225+ details = app.get_details(self.db)
226+ self.navigation_bar.add_with_id(details.display_name,
227+ self.on_navigation_details,
228+ "details",
229+ animate=True)
230 self.app_details.show_app(app)
231 self.display_details()
232
233
234=== modified file 'softwarecenter/view/installedpane.py'
235--- softwarecenter/view/installedpane.py 2010-09-08 01:35:52 +0000
236+++ softwarecenter/view/installedpane.py 2010-09-14 02:59:44 +0000
237@@ -199,7 +199,11 @@
238 """ Display an application in the installed_pane """
239 self.navigation_bar.add_with_id(_("Installed Software"), self.on_navigation_list, "list", do_callback=False, animate=False)
240 self.navigation_bar.remove_all(do_callback=False, animate=False) # do_callback and animate *must* both be false here
241- self.navigation_bar.add_with_id(app.name, self.on_navigation_details, "details", animate=False)
242+ details = app.get_details(self.db)
243+ self.navigation_bar.add_with_id(details.display_name,
244+ self.on_navigation_details,
245+ "details",
246+ animate=False)
247 self.app_details.show_app(app)
248 self.app_view.emit("application-selected", app)
249
250
251=== modified file 'softwarecenter/view/softwarepane.py'
252--- softwarecenter/view/softwarepane.py 2010-09-13 09:14:31 +0000
253+++ softwarecenter/view/softwarepane.py 2010-09-14 02:59:44 +0000
254@@ -249,7 +249,8 @@
255 def on_application_activated(self, appview, app):
256 """callback when an app is clicked"""
257 LOG.debug("on_application_activated: '%s'" % app)
258- self.navigation_bar.add_with_id(app.name,
259+ details = app.get_details(self.db)
260+ self.navigation_bar.add_with_id(details.display_name,
261 self.on_navigation_details,
262 "details")
263 self.notebook.set_current_page(self.PAGE_APP_DETAILS)

Subscribers

People subscribed via source and target branches