Hi list. I would like opinions regarding the code in this method. The
current code (from svn, 1.0 branch):

file paginate.py:

class Paginate:
...
    def get_href(self, page, order=None, reverse_order=None):
        # Note that reverse_order is not used.  It should be cleaned up here
        # and in the template.  I'm not removing it now because I don't want
        # to break the API.
        order = order or None
        self.input_values[self.var_name + '_tgp_no'] = page
        if order:
            self.input_values[self.var_name + '_tgp_order'] = order

        return turbogears.url('', self.input_values)

Now, this code is used in the default template first to output the
pagination links (previous, next, and a range of pages), and then is used to
output the table headers, including the sortable ones. When getting the href
for a sortable column, the inner state of the objects is changed (
self.input_values[self.var_name + '_tgp_order'] is redefined because order
is not None). I think this is unnecessary and it fires a bug if you don't
use the default template and do something as simple as moving the pagination
links below the table (the order is now that set when the header for the
last column of the table was generated, as it was the last change made to
self.input_values).<http://www.google.com/search?hl=en&rlz=1B3GGGL_enAR214AR214&sa=X&oi=spell&resnum=0&ct=result&cd=1&q=unnecessary+&spell=1>

I think something like this should be enough:

    def get_href(self, page, order=None, reverse_order=None):
        # Note that reverse_order is not used.  It should be cleaned up here
        # and in the template.  I'm not removing it now because I don't want
        # to break the API.
        order = order or None
        input_values = self.input_values.copy()
        input_values[self.var_name + '_tgp_no'] = page

        if order:
            input_values[self.var_name + '_tgp_order'] = order

        return turbogears.url('', input_values)

Or as diff

--- paginate.py    (revision 3607)
+++ paginate.py    (working copy)
@@ -320,11 +320,13 @@
         # and in the template.  I'm not removing it now because I don't
want
         # to break the API.
         order = order or None
-        self.input_values[self.var_name + '_tgp_no'] = page
+        input_values = self.input_values.copy()
+        input_values[self.var_name + '_tgp_no'] = page
+
         if order:
-            self.input_values[self.var_name + '_tgp_order'] = order
+            input_values[self.var_name + '_tgp_order'] = order

-        return turbogears.url('', self.input_values)
+        return turbogears.url('', input_values)

 def _select_pages_to_show(current_page, page_count, max_pages):
     pages_to_show = []

If I got this right I'll open a ticket in Trac.

Cheers,
-- 
Juan

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"TurboGears" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/turbogears?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to