Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1414 (closed defect: fixed)

Opened 12 years ago

Last modified 12 years ago

[PATCH] turbogears.url changes tgparams parameter

Reported by: imix Owned by: anonymous
Priority: normal Milestone: 1.0.4
Component: TurboGears Version: 1.0.2
Severity: normal Keywords:
Cc:

Description

The function url in turbogears/controllers.py does the following:

    if tgparams is not None:
        tgparams.update(kw)
    else:
        tgparams = kw

this changes the content of the tgparams dictionary outside the function.

Quoting from jon: ( http://groups.google.com/group/turbogears/browse_thread/thread/300430db80e21517/71caff3e9cc7f23e#71caff3e9cc7f23e)

It seems to me that this is a flat out bug: it should be the other way around, (kw.update(tgparams)) and use kw from that point on.

~jon

Which would result in the following patch:

--- controllers.py.orig 2007-06-26 15:25:08.000000000 +0200
+++ controllers.py      2007-06-26 15:26:31.000000000 +0200
@@ -488,11 +488,9 @@
     else:
         result = tgpath
     if tgparams is not None:
-        tgparams.update(kw)
-    else:
-        tgparams = kw
+        kw.update(tgparams)
     args = []
-    for key, value in tgparams.iteritems():
+    for key, value in kw.iteritems():
         if value is None:
             continue
         if isinstance(value, unicode):

Change History

comment:1 Changed 12 years ago by faide

  • Summary changed from turbogears.url changes tgparams parameter to [PATCH] turbogears.url changes tgparams parameter

comment:2 Changed 12 years ago by Chris Arndt

  • Milestone changed from 1.0.3 to 1.0.4

Batch promoting 1.0.3 tickets to Milestone 1.0.4

comment:3 follow-up: ↓ 4 Changed 12 years ago by Chris Arndt

I think the code should make a copy of tgparams and then update that with kw. The keyword arguments should override tgparams and not the other way round as your patch would do.

    if tgparams is not None:
        tgparams = tgparams.copy()
        tgparams.update(kw)
    else:
        tgparams = kw

comment:4 in reply to: ↑ 3 Changed 12 years ago by imix

You are right, I neglected the case when a name appears both in tgparams and kw. Your solutions seems more logical to me.

comment:5 Changed 12 years ago by Chris Arndt

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in r3368 and r3369 and tests added.

Note: See TracTickets for help on using tickets.