Ticket #7686 (closed enhancement: fixed)

Opened 3 years ago

Last modified 2 years ago

Support `ListOption`s with have a list as default value

Reported by: martin_s Assigned to: rjollos
Priority: normal Component: IniAdminPlugin
Severity: normal Keywords: ListOption
Cc: doki_pen Trac Release: 0.11

Description

The ListOption can work with a comma or otherwise separated string or a list (or similar) as default argument. The underlying getlist is checking the type and handles both cases. Unfortunately the IniAdminPlugin does not handles lists as default value and passes them to Genshi template where they are displayed without the separator.

For example the ctxtnav_names option of the AnnouncerPlugin has a default value of [_('Watch This'),_('Unwatch This')] which is shown as Watch ThisUnwatch This in the panel.

I like to suggest the following changes which resemble the way getlist handles this case: 

Index: iniadminplugin/0.11/iniadmin/iniadmin.py
===================================================================
--- iniadminplugin/0.11/iniadmin/iniadmin.py	(revision 8874)
+++ iniadminplugin/0.11/iniadmin/iniadmin.py	(working copy)
@@ -75,6 +75,9 @@
             value = self.env.config.get(page, option.name)
             # We assume the classes all end in "Option"
             type = option.__class__.__name__.lower()[:-6] or 'text'
+            # Handle list option which have a list as default value
+            if type == 'list' and not isinstance(value,basestring):
+                value = unicode(option.sep).join(list(value))
             option_data  = {'name': option.name, 'default': option.default,
                            'doc': Markup(doc), 'value': value, 'type': type}
             if type == 'extension':

Attachments

Change History

09/15/10 15:30:57 changed by martin_s

  • cc set to doki_pen.
  • keywords set to ListOption.

09/16/10 12:15:00 changed by martin_s

  • owner changed from athomas to rjollos.

09/16/10 12:49:27 changed by rjollos

  • status changed from new to closed.
  • resolution set to fixed.

(In [8899]) Support ListOptions that have a list as a default value. Fixes #7686.

09/16/10 12:55:05 changed by rjollos

Thanks for the patch! I had meant to credit you in the log message, but it slipped my mind while doing the commit.

09/16/10 13:42:45 changed by martin_s

No problem, thanks for applying the patch so quickly.

09/19/10 19:45:49 changed by doki_pen

  • status changed from closed to reopened.
  • resolution deleted.
01:43:36 PM Trac[main] ERROR: Internal Server Error: 
Traceback (most recent call last):
  File "/home/doki_pen/th-announcer/lib/python2.6/site-packages/trac/web/main.py", line 513, in _dispatch_request
    dispatcher.dispatch(req)
  File "/home/doki_pen/th-announcer/lib/python2.6/site-packages/trac/web/main.py", line 235, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/doki_pen/th-announcer/lib/python2.6/site-packages/trac/admin/web_ui.py", line 116, in process_request
    path_info)
  File "/home/doki_pen/th-announcer/lib/python2.6/site-packages/iniadmin/iniadmin.py", line 79, in render_admin_panel
    value = unicode(option.sep).join(list(value))

09/19/10 19:56:03 changed by martin_s

The error messages from the log file would be really helpful here.

(follow-up: ↓ 11 ) 09/19/10 20:03:17 changed by doki_pen

  • status changed from reopened to closed.
  • resolution set to invalid.

I figured it out. It's because we had a ListOption? defined with a default value of:

[_('Watch This'), _('Unwatch This')], which doesn't make much sense. babel LazyProxy? objects where passed into the join, which doesn't work. The correct solution was to wrap the strings in _() before they are rendered, instead of in the ListOption? definition.

09/19/10 20:03:26 changed by doki_pen

  • status changed from closed to reopened.
  • resolution deleted.

09/19/10 20:03:32 changed by doki_pen

  • status changed from reopened to closed.
  • resolution set to fixed.

(in reply to: ↑ 8 ) 09/19/10 20:14:01 changed by martin_s

Replying to doki_pen:

I figured it out. It's because we had a ListOption? defined with a default value of: [_('Watch This'), _('Unwatch This')], which doesn't make much sense. babel LazyProxy? objects where passed into the join, which doesn't work. The correct solution was to wrap the strings in _() before they are rendered, instead of in the ListOption? definition.

Ok, makes sense. But you still need to mark them for the message extractor. I think the N_ function does this. AFAIK it returns the string unchanged but the message extractor will acknowledge it. As you mentioned later when the navigation items are rendered they must be processed with _ or directly gettext. I do this with the option doc texts in the dev branch of the WatchlistPlugin.

Add/Change #7686 (Support `ListOption`s with have a list as default value)




Change Properties
Action