Modify

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#7686 closed enhancement (fixed)

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

Reported by: martin_s Owned by: 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:

  • iniadminplugin/0.11/iniadmin/iniadmin.py

     
    7575            value = self.env.config.get(page, option.name) 
    7676            # We assume the classes all end in "Option" 
    7777            type = option.__class__.__name__.lower()[:-6] or 'text' 
     78            # Handle list option which have a list as default value 
     79            if type == 'list' and not isinstance(value,basestring): 
     80                value = unicode(option.sep).join(list(value)) 
    7881            option_data  = {'name': option.name, 'default': option.default, 
    7982                           'doc': Markup(doc), 'value': value, 'type': type} 
    8083            if type == 'extension': 

Attachments (0)

Change History (11)

comment:1 Changed 4 years ago by martin_s

  • Cc doki_pen added
  • Keywords ListOption added

comment:2 Changed 4 years ago by martin_s

  • Owner changed from athomas to rjollos

comment:3 Changed 4 years ago by rjollos

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

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

comment:4 Changed 4 years ago 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.

comment:5 Changed 4 years ago by martin_s

No problem, thanks for applying the patch so quickly.

comment:6 Changed 4 years ago by doki_pen

  • Resolution fixed deleted
  • Status changed from closed to reopened
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))

comment:7 Changed 4 years ago by martin_s

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

comment:8 follow-up: Changed 4 years ago by doki_pen

  • Resolution set to invalid
  • Status changed from reopened to closed

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.

comment:9 Changed 4 years ago by doki_pen

  • Resolution invalid deleted
  • Status changed from closed to reopened

comment:10 Changed 4 years ago by doki_pen

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

comment:11 in reply to: ↑ 8 Changed 4 years ago 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 Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.