[asterisk-dev] [Code Review]: Add xmldoc for config options, along with some CLI commands for viewing config option-related stuff
Mark Michelson
reviewboard at asterisk.org
Thu Jul 19 13:38:10 CDT 2012
> On July 19, 2012, 12:56 p.m., Mark Michelson wrote:
> > The main issue I'm having here is that I find the XML schema for configuration to be clunky and unnatural. Instead of defining configObjects in a list and then configOptions that refer to those configObjects, it would make sense to use a hierarchical approach. To give an example of what I mean, here's a potential representation for app_skel's "level" config.
> >
> > <configInfo module="app_skel" language="en_US">
> > <configObject name="level">
> > <synopsis>Defined levels for the SkelGuessNumber game</synopsis>
> > <syntax><category match="false">^(general|sounds)$</category></syntax>
> > <configOption name="max_number">
> > <synopsis>The maximum in the range of numbers to guess (1 is the implied minimum)</synopsis>
> > <syntax><dataType type="uint"/></syntax>
> > </configOption>
> > <configOption name="max_guesses">
> > <synopsis>The maximum number of guesses before a game is considered lost</synopsis>
> > <syntax><dataType type="uint"/></syntax>
> > </configOption>
> >
> > In my opinion this style of grouping is easier to comprehend.
> >
> > As far as your actual code goes, I'm trying to steer clear of too-specific comments, but it would be good for CLI commands to be able to tab-complete items such as modules, types, and perhaps options.
>
> Terry Wilson wrote:
> The reason I did it this way is that the underlying structure allows an option to be defined for multiple types, for example allow/disallow is good for both SIP peers and users. The hierarchical approach would force duplication.
>
> Yeah, the CLI functions need tab-completion, and to not be fugly. I'll certainly get to that if the rest of the stuff pans out.
I had thought perhaps the config object applying to multiple sections might be the reason you did it this way. I still contend that having it hierarchical is possible and better.
You can either
1) Use xi:include or some other such referencing method in order to include a common config option within a category. So for instance, you could define a configOption called "allow" outside of a configObject. Then within the configObject, just include the configOption.
2) Use your method only for options that fall outside of one specific category.
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2058/#review6744
-----------------------------------------------------------
On July 18, 2012, 7:46 p.m., Terry Wilson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2058/
> -----------------------------------------------------------
>
> (Updated July 18, 2012, 7:46 p.m.)
>
>
> Review request for Asterisk Developers and Matt Jordan.
>
>
> Summary
> -------
>
> This patch kind of shoe-horns config option documentation into the current xmldoc stuff. It is currently proof-of-concept level code and needs to be cleaned up. I'd really like some feedback on how this should really go. I've changed my mind and re-written things several times now.
>
> What it does:
> 1. Uses the ast_xml_doc_item stuff for manager events to store synopsis, description, and very limited syntax information about config items
> 2. Adds CLI functions config show option(s)/type(s) to display some limited info from the XML docs
> 3. Checks that, if XML_DOCS are enabled, that there is XML documentation for an option being registered (it currently doesn't return -1 if missing since I've only added docs for app_skel)
>
> What it still needs:
> 1. It needs to record more information that is available in the xmldocs in the ast_xml_doc_item (defaults, type fields for objects, valid ranges for values, etc.)
> 2. The CLI output is ugly and just mostly place-holder text displaying the values. It needs to be made more consistent with other output.
> 3. More descriptions should be added with the appropriate formatting, etc.
>
> What I wish we were doing instead:
> 1. Writing little parsers of the XML data to store raw text in the ast_xml_doc_item to output to the CLI is...suboptimal. We have XML data. We should use XSLT to transform the XML data into CLI output, manager output, or whatever we need. This would also make it easier to make language-neutral output since the "syntax" info relies on things like the word "category", whether or not a regex matches or does not match, type fields, etc.
>
> 2. Some information is duplicated in the XML docs and in the aco_option_register() arguments. xmldocs are optional, so you can't remove the arguments from aco_option_register. Having aco_option_register generate the XML and apply it to the in-memory xmldocs at runtime with a command to dump the updated XML would be a solution to removing the duplication and still being able to generate complete xml documentation for translating to the wiki, etc.
>
> This is an attempt to get something in for Asterisk 11, even if it isn't (anywhere near) perfect. I just don't want to spend even more time cleaning up stuff if we decide to change the design a lot. There are some red blobs that need cleaning up, etc. as well.
>
>
> Diffs
> -----
>
> /trunk/apps/app_skel.c 370233
> /trunk/doc/appdocsxml.dtd 370233
> /trunk/include/asterisk/_private.h 370233
> /trunk/include/asterisk/config_options.h 370233
> /trunk/include/asterisk/xmldoc.h 370233
> /trunk/main/asterisk.c 370233
> /trunk/main/config_options.c 370233
> /trunk/main/xmldoc.c 370233
>
> Diff: https://reviewboard.asterisk.org/r/2058/diff
>
>
> Testing
> -------
>
> tested CLI commands.
>
>
> Thanks,
>
> Terry
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120719/b188970c/attachment.htm>
More information about the asterisk-dev
mailing list