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

[Gary Lasker]

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!

[Gary Lasker (gary-lasker)]

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).

[lp:~gary-lasker/software-center/title-summary-fixes]

1168. By Gary Lasker on 2010-09-13

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

[Gary Lasker (gary-lasker)]

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.)

[lp:~gary-lasker/software-center/title-summary-fixes]

1169. By Gary Lasker on 2010-09-14

merge with trunk

=== modified file 'debian/changelog'
--- debian/changelog	2010-09-13 16:27:49 +0000
+++ debian/changelog	2010-09-14 02:59:44 +0000
@@ -6,6 +6,16 @@
   * softwarecenter/backend/channel.py:
     - always display the partner channel, even if its
       source is not enabled (LP: #635003) 
+  * softwarecenter/db/application.py,
+    softwarecenter/db/database.py,
+    softwarecenter/view/appview.py,
+    softwarecenter/view/softwarepane.py:
+    - make StoreDatabase.get_appname return None if the
+      name is not defined in the xapian doc
+    - consolidate code to generate application name and
+      summary for display in the UI to a single place
+    - wire applist, appdetails and the navigation bar
+      to use it (LP: #636004)
 
   [ Kiwinote ]
   * data/software-center.menu.in:
@@ -14,9 +24,12 @@
     - allow 'for purchase' to have an icon in non-English locales (LP: #636242)
   * softwarecenter/db/application/py:
     - try to open deb file, except on any sort of error (LP: #633379)
+    - reload pkg cache object and xapian doc if it turns out that the pkgname
+      of a deb file is different than what we guessed on basis of the file name
   * softwarecenter/view/appdetailsview_gtk.py:
     - use the display name for app entry in 'where is it' (LP: #635464)
     - restore correct padding in addons status bar (LP: #625230)
+  * minor tweaks to lp:~gary-lasker/software-center/title-summary-fixes
 
   [ Matthew Paul Thomas ]
   * data/software-center.menu.in:
@@ -26,7 +39,7 @@
   [ Colin Watson ]
   * Fix update-apt-xapian-index crashes when the Dir::Cache::pkgcache file
     doesn't exist (LP: #267330).
-  
+    
   [ Michael Vogt ]
   * softwarecenter/backend/aptd.py:
     - fix "reload()" in enable_channel() and only update the particular
@@ -36,8 +49,6 @@
   * merged lp:~mmcg069/software-center/Bug635375
   * merged  lp:~mmcg069/software-center/tiny-pathbar-tweak
 
- -- Michael Vogt <[e-mail]>  Mon, 13 Sep 2010 16:18:45 +0200
-
 software-center (2.1.18.1) maverick; urgency=low
 
   [ Colin Watson ]
 
=== modified file 'softwarecenter/db/application.py'
--- softwarecenter/db/application.py	2010-09-11 14:49:38 +0000
+++ softwarecenter/db/application.py	2010-09-14 02:59:44 +0000
@@ -69,6 +69,28 @@
     def get_details(self, db):
         """ return a new AppDetails object for this application """
         return AppDetails(db, application=self)
+    def get_display_name(self, db, doc):
+        """ Return the application name as it should be displayed in the UI
+            If the appname is defined, just return it, else return
+            the summary (per the spec)
+        """
+        if doc:
+            appname = db.get_appname(doc)
+            if appname:
+                return appname
+            else:
+                return db.get_summary(doc)
+    def get_display_summary(self, db, doc):
+        """ Return the application summary as it should be displayed in the UI
+            If the appname is defined, return the application summary, else return
+            the application's pkgname (per the spec)
+        """
+        if doc:
+            if db.get_appname(doc):
+                return db.get_summary(doc)
+            else:
+                return db.get_pkgname(doc)
+        
     # special methods
     def __hash__(self):
         return ("%s:%s" % (self.appname, self.pkgname)).__hash__()
@@ -96,7 +118,7 @@
         if not debfile.endswith(".deb") and not debfile.count('/') >= 2:
             raise ValueError("Need a deb file, got '%s'" % debfile)
         debname = os.path.splitext(os.path.basename(debfile))[0]
-        self.appname = debname.split('_')[0].capitalize()
+        self.appname = ""
         self.pkgname = debname.split('_')[0].lower()
         self.request = debfile
     def get_details(self, db):
@@ -295,38 +317,24 @@
     
     @property
     def display_name(self):
-        """ Return the name as it should be displayed in the UI
-
-            Note that this may not corespond to the Appname as the
-            spec says the name should be the summary for packages
-            and the summary the pkgname
+        """ Return the application name as it should be displayed in the UI
+            If the appname is defined, just return it, else return
+            the summary (per the spec)
         """
         if self._error_not_found:
             return self._error
         if self._doc:
-            name = self._db.get_appname(self._doc)
-            if name:
-                return name
-            else:
-                # by spec..
-                return self._db.get_summary(self._doc)
+            return self._app.get_display_name(self._db, self._doc)
         return self.name
 
     @property
     def display_summary(self):
-        """ Return the summary as it should be displayed in the UI
-
-            Note that this may not corespond to the summary value as the
-            spec says the name should be the summary for packages
-            and the summary the pkgname
+        """ Return the application summary as it should be displayed in the UI
+            If the appname is defined, return the application summary, else return
+            the application's pkgname (per the spec)
         """
         if self._doc:
-            name = self._db.get_appname(self._doc)
-            if name:
-                return self._db.get_summary(self._doc)
-            else:
-                # by spec..
-                return self._db.get_pkgname(self._doc)
+            return self._app.get_display_summary(self._db, self._doc)
         return ""
 
     @property
@@ -592,9 +600,24 @@
                     self._error_not_found = _("The file \"%s\" could not be opened.") % self._app.request
             return
 
-        if self.pkgname:
+        if self.pkgname and self.pkgname != self._app.pkgname:
+            # this happens when the deb file has a quirky file name
             self._app.pkgname = self.pkgname
 
+            # load pkg cache
+            self._pkg = None
+            if (self._app.pkgname in self._cache and 
+                self._cache[self._app.pkgname].candidate):
+                self._pkg = self._cache[self._app.pkgname]
+ 
+            # load xapian document
+            self._doc = None
+            try:
+                self._doc = self._db.get_xapian_document(
+                    self._app.appname, self._app.pkgname)
+            except:
+                pass
+
         # check deb and set failure state on error
         if not self._deb.check():
             self._error = self._deb._failure_string
 
=== modified file 'softwarecenter/db/database.py'
--- softwarecenter/db/database.py	2010-09-10 09:17:07 +0000
+++ softwarecenter/db/database.py	2010-09-14 02:59:44 +0000
@@ -223,19 +223,16 @@
         """ Return a packagename from a xapian document """
         pkgname = doc.get_value(XAPIAN_VALUE_PKGNAME)
         # if there is no value it means we use the apt-xapian-index 
-        # that store the pkgname in the data field directly
+        # that stores the pkgname in the data field directly
         if not pkgname:
             pkgname = doc.get_data()
         return pkgname
 
     def get_appname(self, doc):
-        """ Return a appname from a xapian document """
-        appname = doc.get_value(XAPIAN_VALUE_APPNAME)
-        # if there is no value it means we use the apt-xapian-index 
-        # and that has no appname
-        if not appname:
-            appname = doc.get_data()
-        return appname
+        """ Return a appname from a xapian document, or None if
+            a value for appname cannot be found in the document
+         """
+        return doc.get_value(XAPIAN_VALUE_APPNAME)
 
     def get_iconname(self, doc):
         """ Return the iconname from the xapian document """
 
=== modified file 'softwarecenter/view/appview.py'
--- softwarecenter/view/appview.py	2010-09-10 09:17:07 +0000
+++ softwarecenter/view/appview.py	2010-09-14 02:59:44 +0000
@@ -540,13 +540,8 @@
             summary = self.db.get_summary(doc)
             return "%s\n%s" % (appname, summary)
         elif column == self.COL_MARKUP:
-            appname = app.appname
-            summary = self.db.get_summary(doc)
-            # SPECIAL CASE: the spec says that when there is no appname, 
-            #               the summary should be displayed as appname
-            if not appname:
-                appname = summary
-                summary = app.pkgname
+            appname = app.get_display_name(self.db, doc)
+            summary = app.get_display_summary(self.db, doc)
             if self.db.is_appname_duplicated(appname):
                 appname = "%s (%s)" % (appname, app.pkgname)
             s = "%s\n<small>%s</small>" % (
 
=== modified file 'softwarecenter/view/availablepane.py'
--- softwarecenter/view/availablepane.py	2010-09-13 09:14:31 +0000
+++ softwarecenter/view/availablepane.py	2010-09-14 02:59:44 +0000
@@ -486,7 +486,11 @@
         else:
             self.apps_category = Category("deb", "deb", None, None, False, True, None)
         self.current_app_by_category[self.apps_category] = app
-        self.navigation_bar.add_with_id(app.name, self.on_navigation_details, "details", animate=True)
+        details = app.get_details(self.db)
+        self.navigation_bar.add_with_id(details.display_name,
+                                        self.on_navigation_details,
+                                        "details",
+                                        animate=True)
         self.app_details.show_app(app)
         self.display_details()
 
 
=== modified file 'softwarecenter/view/installedpane.py'
--- softwarecenter/view/installedpane.py	2010-09-08 01:35:52 +0000
+++ softwarecenter/view/installedpane.py	2010-09-14 02:59:44 +0000
@@ -199,7 +199,11 @@
         """ Display an application in the installed_pane """
         self.navigation_bar.add_with_id(_("Installed Software"), self.on_navigation_list, "list", do_callback=False, animate=False)
         self.navigation_bar.remove_all(do_callback=False, animate=False) # do_callback and animate *must* both be false here
-        self.navigation_bar.add_with_id(app.name, self.on_navigation_details, "details", animate=False)
+        details = app.get_details(self.db)
+        self.navigation_bar.add_with_id(details.display_name,
+                                        self.on_navigation_details,
+                                        "details",
+                                        animate=False)
         self.app_details.show_app(app)
         self.app_view.emit("application-selected", app)
 
 
=== modified file 'softwarecenter/view/softwarepane.py'
--- softwarecenter/view/softwarepane.py	2010-09-13 09:14:31 +0000
+++ softwarecenter/view/softwarepane.py	2010-09-14 02:59:44 +0000
@@ -249,7 +249,8 @@
     def on_application_activated(self, appview, app):
         """callback when an app is clicked"""
         LOG.debug("on_application_activated: '%s'" % app)
-        self.navigation_bar.add_with_id(app.name,
+        details = app.get_details(self.db)
+        self.navigation_bar.add_with_id(details.display_name,
                                        self.on_navigation_details,
                                        "details")
         self.notebook.set_current_page(self.PAGE_APP_DETAILS)