Merge lp:~gary-lasker/software-center/remember-sort-preference-lp966878 into lp:software-center

[Gary Lasker]

This is a fix for bug 966878. A simple fix to not reset the user's sorting preference if they have set one. We should respect the setting when moving around the UI, as we already do when searching in the categories screens. Please check bug 966878 for details and steps to reproduce.

Thanks!

[Michael Vogt (mvo)]

Thanks for your branch! I like the idea of going for a simple fix, unfortunately this seems to only
fix parts of the issue as e.g.:
- searching for "foo"
- change "by name"
- go back
- go a a category like "accessories" will sort by name, but the combo will show "by top rated"
(note that this is not a regression, but a different incarnation of the bug or a different bug that is very similar to this one).

As for the bit:
...
8 + sortmode=self.get_sort_mode(),
...
I am not sure that this is needed, the name of the function is certainly very misleading, my suggestion would
be to do something like (suggestions for better names welcome of course):
=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py 2012-03-30 09:46:20 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py 2012-04-10 09:37:54 +0000
@@ -618,7 +618,7 @@
             self.refresh_apps()

         query = self.get_query()
- n_matches = self.quick_query(query)
+ n_matches = self.quick_query_len(query)
         self.subcategories_view.set_subcategory(category, n_matches)

         self.action_bar.clear()

=== modified file 'softwarecenter/ui/gtk3/panes/softwarepane.py'
--- softwarecenter/ui/gtk3/panes/softwarepane.py 2012-04-04 14:40:09 +0000
+++ softwarecenter/ui/gtk3/panes/softwarepane.py 2012-04-10 09:34:25 +0000
@@ -431,7 +431,10 @@
         self.show_appview_spinner()
         self._refresh_apps_with_apt_cache(query)

- def quick_query(self, query):
+ def quick_query_len(self, query):
+ """ do a blocking query that only returns the amount of
+ matches from this query
+ """
         # a blocking query and does not emit "query-complete"
         with ExecutionTime("enquirer.set_query() quick query"):
             self.enquirer.set_query(

To make it clearer what its actually doing. So unless I miss something, the sort order should not
influence the amount of items returned, so it should be ok to omit it.

[Michael Vogt (mvo)]

Actually I just noticed (or remembered) that the first bit I described above is probably very similar (or the same?) as bug #969215 so this part may not be relevant, if so, sorry for the noise.

[Gary Lasker (gary-lasker)]

Hi Michael, and thanks for your comments! Yes, I believe that the other case you describe is a symptom of the separate fix needed for bug 969215 (working on that now).

> As for the bit:
> ...
> 8 + sortmode=self.get_sort_mode(),

Indeed, that was just something that I tried quickly early on and then inadvertently left in the branch. This can go (and the method renamed for clarity is a good idea as well).

[Gary Lasker (gary-lasker)]

I've update my branch for the above. Thanks Michael!

[lp:~gary-lasker/software-center/remember-sort-preference-lp966878]

2948. By Gary Lasker on 2012-04-10

updated per comments from mvo in the merge proposal, thanks Michael

[Michael Vogt (mvo)]

Thanks for this update. I'm happy to merge it, the only downside I see is that once the sort method
was changed it will always stay changed. I.e. never jump back to the "default" of the page. I wonder
if its maybe worthwhile to add this to the view_state we keep? I will merge the branch as it is and
investigate that other option a bit.

review: Approve

[Gary Lasker (gary-lasker)]

It actually does switch back to the default of the page when it is needed, afaict, such as when viewing the lists from the "More" buttons in the lobby view. I could not find a place where I didn't want my selected to be simply remembered. But, it's worth more investigation certaintly.

I find not resetting it to default constantly to be a big improvement, personally. :)

[Michael Vogt (mvo)]

On Tue, Apr 10, 2012 at 04:21:23PM -0000, Gary Lasker wrote:
> It actually does switch back to the default of the page when it is needed, afaict, such as when viewing the lists from the "More" buttons in the lobby view. I could not find a place where I didn't want my selected to be simply remembered. But, it's worth more investigation certaintly.
>
> I find not resetting it to default constantly to be a big improvement, personally. :)

Thanks, you are right :) I don't quite understand this in the code, but
the effect is really nice. When switching back and forth I get my sort
method preserved, but when switching to categories or similar I get
the default. Its a bit of a mystery why this is given that I can't see
any bit in the code that resets user_definied_sort_method once its
changed, but I'm too tired today to think about this more.

Thanks again!
 Michael

[Gary Lasker (gary-lasker)]

Indeed, the code needs some cleanup for sure, but at this late stage in the cycle I'm not sure we want to rework these pieces. The reason categories act differently is because the code path is separate and does not use user_defined_sort_method. That's part of what I think we want to fix, we actually do not want to even have to use this variable imo.

So, I think we just need to define clearly what we want sorting to do in each view (the spec probably says this already), and then do that with the simplest implementation possible. As a certain wise man I know has said on many an occasion, it's just a simple matter of programming!

:)

[Michael Vogt (mvo)]

Thanks! I agree with that.

=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py	2012-03-30 09:46:20 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py	2012-04-10 16:09:21 +0000
@@ -618,7 +618,7 @@
             self.refresh_apps()
 
         query = self.get_query()
-        n_matches = self.quick_query(query)
+        n_matches = self.quick_query_len(query)
         self.subcategories_view.set_subcategory(category, n_matches)
 
         self.action_bar.clear()

=== modified file 'softwarecenter/ui/gtk3/panes/softwarepane.py'
--- softwarecenter/ui/gtk3/panes/softwarepane.py	2012-04-04 14:40:09 +0000
+++ softwarecenter/ui/gtk3/panes/softwarepane.py	2012-04-10 16:09:21 +0000
@@ -431,8 +431,10 @@
         self.show_appview_spinner()
         self._refresh_apps_with_apt_cache(query)
 
-    def quick_query(self, query):
-        # a blocking query and does not emit "query-complete"
+    def quick_query_len(self, query):
+        """ do a blocking query that only returns the amount of
+            matches from this query
+        """
         with ExecutionTime("enquirer.set_query() quick query"):
             self.enquirer.set_query(
                                 query,

=== modified file 'softwarecenter/ui/gtk3/views/appview.py'
--- softwarecenter/ui/gtk3/views/appview.py	2012-04-10 05:05:22 +0000
+++ softwarecenter/ui/gtk3/views/appview.py	2012-04-10 16:09:21 +0000
@@ -210,7 +210,6 @@
         if model:
             model.set_from_matches(matches)
         self.set_model(model)
-        self.user_defined_sort_method = False
 
         self.tree_view_scroll.get_vadjustment().set_lower(self.vadj)
         self.tree_view_scroll.get_vadjustment().set_value(self.vadj)