[Patch] No documentation on the TracIni page

[Ryan J Ollos]

The TracJsGanttPlugin currently has no documentation on the TracIni page. I'll upload my patch in a moment.

[Ryan J Ollos]

This is the documentation we'll get on TracIni with the patch. Only one of the docstring has been filled in.

A possibility for keeping the wiki page documentation up to date would be to just do a screen capture like this and paste it on the wiki page.

[Ryan J Ollos]

Here is the patch. The current status is:

• Most of the documentation strings need to be filled in.
• I haven't done extensive testing to check that all of the options are still functional and that I got the data types correct.
• On line 72 there was a conditional if v != self: that I dropped because I couldn't see its purpose and I would need to understand the condition it is handling to properly include it in the patch.

Btw, I had also planned to work on #8989 and #8948 this weekend, but it looks like I won't get to them until next weekend. I will have time during the week to work on this ticket's patch though, and I'm posting it now because I wanted to get some immediate feedback as to whether this could be incorporated before I went typing up all the documentation strings and extensively testing.

[Patrick Schaaf]

Replying to rjollos:

• On line 72 there was a conditional if v != self: that I dropped because I couldn't see its purpose and I would need to understand the condition it is handling to properly include it in the patch.

It is not neccessary when using the trac Option() style approach your patch employs. The check goes along with the default=self specification on the preceding line, as an (as I thought) obvious and correct way to handle "that option was not present in trac.ini - use the self.options[] default."

Your patch does not go along well with the pending patch I did wrt using my new TracMacroConfigPlugin helper, to make the macro options into "named config=urable and inheritable entities. See #8979 for reference. My approach relies, as of now, on a separate dict of macro options, and computationally derives the real option names from that dict's keys.

Of course that might just mean that my approach is not the best... Maybe you could have a look at it (its intention), and have some useful suggestions about how to evolve it, either as comments on #8979, or a new ticket on TracMacroConfigPlugin.

[Ryan J Ollos]

Replying to bof:

It is not neccessary when using the trac Option() style approach your patch employs. The check goes along with the default=self specification on the preceding line, as an (as I thought) obvious and correct way to handle "that option was not present in trac.ini - use the self.options[] default."

You are right, it was fairly clear ... I just wasn't thinking very sharp when reading that code!

Your patch does not go along well with the pending patch I did wrt using my new TracMacroConfigPlugin helper, to make the macro options into "named config=urable and inheritable entities. See #8979 for reference. My approach relies, as of now, on a separate dict of macro options, and computationally derives the real option names from that dict's keys.

Of course that might just mean that my approach is not the best... Maybe you could have a look at it (its intention), and have some useful suggestions about how to evolve it, either as comments on #8979, or a new ticket on TracMacroConfigPlugin.

Maybe we just use the Option style with trac.ini options that aren't macro argument defaults? It might be more useful to somehow include your argument configurations on the WikiMacros page, since that is where a user goes for help (whereas an administrator is typically the only person to visit the TracIni page).

If I have any more useful thoughts I'll post them to #8979. Thanks for your input!

[Ryan J Ollos]

Replying to bof:

Your patch does not go along well with the pending patch I did wrt using my new TracMacroConfigPlugin helper, to make the macro options into "named config=urable and inheritable entities. See #8979 for reference. My approach relies, as of now, on a separate dict of macro options, and computationally derives the real option names from that dict's keys.

As I look at this more, it seems like the patches could work together in a clean way. Define all of the options using the Option style, which takes care of the default value if a parameter is not specified in trac.ini. Then modify your macro call slightly:

self.macroconfig = tracmacroconfig.TracMacroConfig( 
                self.config, 'trac-jsgantt', 
                defaultprefix = 'option', optionspec = { 
                    # All the macro's options, with default values. 
                    # Anything else passed to the macro is a TracQuery field. 
                    'format': self.config.get('trac-jsgantt', 'day'), 
                        'sample': self.config.get('trac-jsgantt', 'sample'),  
                        ...

Does that seem like it will work?

[Patrick Schaaf]

Replying to rjollos:

As I look at this more, it seems like the patches could work together in a clean way. Define all of the options using the Option style, which takes care of the default value if a parameter is not specified in trac.ini. Then modify your macro call slightly: [SNIP]

Does that seem like it will work?

No, it wouldn't, that way.

The "default" options in trac.ini are called option.format etc. It could work if the Option() statements would use these fully qualified names. The TracMacroConfig instantiation call could then probably also just use the self.option_format etc. variables for initialization.

What I'm thinking about, is learning a bit more about python descriptors - I'm a bloody newbie at python, actually - and then provide in TracMacroConfig, suitable descriptor classes mirroring the Option/BoolOption/... scheme, e.g. MacroOption/MacroBoolOption/...

Something like this:

from tracmacroconfig import TracMacroConfig, MacroOption, MacroIntOption

class TracJSGanttChart(WikiMacroBase):
    macroconfig = TracMacroConfig('trac-jsgantt', defaultprefix = 'option')
    option_format = MacroOption(macroconfig, 'format', 'day')
    option_openLevel = MacroIntOption(macroconfig, 'openLevel', 999)
    # ...
    # NOTHING about that in __init__
    # ... then later
    def expand_macro(self, formatter, name, content):
        query_options = self.macroconfig.use(content)
        # NOW, query_options only_ has the options to pass to ticket query
        # AND now access to self.option_format and friends, will be bound to
        # the correct values as determined by TracMacroConfig prefixing/inheritance
        tasks = self._add_tasks(query_options)
        # ...

However one thing makes me nervous. Is there a python/trac deployment scenario where real multithreading is going on at the interpreter level? Could several expand_macro() calls be going on at the same time, in parallel? That would make this approach fail miserably, because access to the self.option_format & friends should return the values calculated on the most recent self.macroconfig.use() call, and parallelism would be chaos!

[Chris Nelson]

I barely understand what you guys are talking about but I have this mostly merged. Should I go ahead and push it?

[Chris Nelson]

I also noted that TracWikiMacros says

[[TracJSGanttChart]]

None

how do I fix that. Might as well get all the docs in one change.

[Ryan J Ollos]

Replying to ChrisNelson:

how do I fix that. Might as well get all the docs in one change.

You just need to put a docstring directly below class TracJSGanttChart(WikiMacroBase):, for example:

class TracJSGanttChart(WikiMacroBase):
    """Usage: ...
    {{{
    [[TracJSGanttChart( ...
    }}}
    """

Taking care of that would resolve #8948.

[Ryan J Ollos]

Replying to ChrisNelson:

I barely understand what you guys are talking about but I have this mostly merged. Should I go ahead and push it?

I'm in favor of pushing it, and then figuring out how to incorporate #8979.

[Patrick Schaaf]

Replying to rjollos:

Replying to ChrisNelson:

I barely understand what you guys are talking about but I have this mostly merged. Should I go ahead and push it?

I'm in favor of pushing it, and then figuring out how to incorporate #8979.

I did some work already wrt my proposal of MacroOption etc. classes. The idea seems to be practicable. I will massage it a bit more in the next hours, push it to the SVN, and provide a new patch in #8979 then.

[Patrick Schaaf]

Sorry, but as usual, programming takes longer once you start on it... I'll try to commit something today in the evening.

Here's a teaser - and it already works, really :)

"""An example macro using TracMacroConfig, called [[TracMacroConfigExample]]

The macro does nothing exciting: it just displays the parameters it was
called with.
"""

from genshi.builder import tag
from trac.wiki.macros import WikiMacroBase

from tracmacroconfig import *

class TracMacroConfigExample(WikiMacroBase):

    mc = TracMacroConfig('macroconfig-example')

    mo_text = mc.Option('text', default='exampletext',
                    doc='''A string macro option''')

    mo_bool = mc.BoolOption('bool', default=True,
                    doc='''A bool macro option''')

    mo_int = mc.IntOption('int', default=42,
                    doc='''An integer macro option''')

    mo_list = mc.ListOption('list', default=['first', 'last'], sep='|',
                    doc='''A list macro option''')

    mc.InheritOption('inherit', doc='''Inherit options from that list''')

    def __init__(self):
        self.mc.setup(self.config)

    def expand_macro(self, formatter, name, arguments):
        self.mc.options(arguments)
        return tag.ul(
            tag.li('An invocation of the %s macro' % name),
            tag.li('Raw arguments: "%s"' % arguments),
            tag.li('Resolved option values:'),
            tag.ul(
                self._show_option(self.mo_text, TracMacroConfigExample.mo_text),
                self._show_option(self.mo_bool, TracMacroConfigExample.mo_bool),
                self._show_option(self.mo_int, TracMacroConfigExample.mo_int),
                self._show_option(self.mo_list, TracMacroConfigExample.mo_list)
            )
        )

    def _show_option(self, option, spec):
        return tag.ul(
          tag.li('Name: %s' % spec.name),
          tag.li('Value: %s' % option),
          tag.li('Default: %s' % str(spec.default)),
          tag.li('Doc: %s' % spec.__doc__)
        )

[Patrick Schaaf]

I think I now got something. Committed to the TracMacroConfigPlugin SVN for you to review. The wiki page needs updating...

There's now an example macro that you can enable through the plugin admin panel. That macro implementation should also give a good intuition about how other macros would use the functionality.

Options defined this way do NOT show up in IniAdmin. I had some problems with that, part of which would result in broken or undesirable trac.ini, and I think this kind of configuration needs a separate MacroIniAdmin interface anyway to capture the spirit of inheritance.

I tested this under 0.11.7 and 0.12.2. Appears to work both ways.

I would love you two to give it a good review!

To test with the example macro I use this trac.ini segment:

[components]
tracmacroconfig.examplemacro.tracmacroconfigexample = enabled

[macroconfig-example]
classa.bool = True
classa.text = classA
classb.bool = False
classb.int = 666
classc.config = classA|classB
macro.list = foo|bar
test.bool = False
test.list = a|b|c
test.text = hello world!
toast.bool = True
toast.config = test
toast.int = 137

And I put something like this on a test wiki page:

[[TracMacroConfigExample()]]
[[TracMacroConfigExample(int=23)]]
[[TracMacroConfigExample(config=test,text=from macro arg)]]
[[TracMacroConfigExample(config=toast)]]
[[TracMacroConfigExample(config=nix, extra=hullo)]]
[[TracMacroConfigExample(config=classA, bool=off)]]
[[TracMacroConfigExample(config=classA|classB)]]
[[TracMacroConfigExample(config=classC)]]

[Patrick Schaaf]

In #8979 there is now a refreshed patch using the descriptor style TracMacroConfig enhancements.

That patch is orthogonal to the work of rjollos here, which concerns itself with the base (non-macro-argument) options of the module.

Chris, please consider merging both.

[Chris Nelson]

(In [10569]) Add configuration field documentation to TracIni page. Refs #8999.

Adapted from a patch by rjollos.

[Ryan J Ollos]

Replying to bof:

I would love you two to give it a good review!

Thanks, I intend to take a look. Probably won't happen until next week though the ways things are going at work ;)

[Chris Nelson]

Replying to rjollos:

Replying to bof:

I would love you two to give it a good review!

Thanks, I intend to take a look. Probably won't happen until next week though the ways things are going at work ;)

I have a deadline this week followed by a week on the beach. I'm likely not going to touch Trac development until the end of the month.

[Patrick Schaaf]

Take your time with the reviewing. I'll be on holiday for the next 2 1/2 weeks, and continue working on trac stuff in September.