Modify

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#7686 closed enhancement (fixed)

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

Reported by: Martin Scharrer Owned by: Ryan J Ollos
Priority: normal Component: IniAdminPlugin
Severity: normal Keywords: ListOption
Cc: Robert Corsaro 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 7 years ago by Martin Scharrer

Cc: Robert Corsaro added; anonymous removed
Keywords: ListOption added

comment:2 Changed 7 years ago by Martin Scharrer

Owner: changed from Alec Thomas to Ryan J Ollos

comment:3 Changed 7 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

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

comment:4 Changed 7 years ago by Ryan J Ollos

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 7 years ago by Martin Scharrer

No problem, thanks for applying the patch so quickly.

comment:6 Changed 7 years ago by Robert Corsaro

Resolution: fixed
Status: closedreopened
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 7 years ago by Martin Scharrer

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

comment:8 Changed 7 years ago by Robert Corsaro

Resolution: invalid
Status: reopenedclosed

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 7 years ago by Robert Corsaro

Resolution: invalid
Status: closedreopened

comment:10 Changed 7 years ago by Robert Corsaro

Resolution: fixed
Status: reopenedclosed

comment:11 in reply to:  8 Changed 7 years ago by Martin Scharrer

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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.