-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Robert Ramey Sent: Thursday, May 05, 2011 3:22 PM To: boost@lists.boost.org Subject: Re: [boost] [review] Formal review of AutoIndex starts 5th May

Daniel James wrote:

...

Hello everyone,

The formal review of John Maddock's AutoIndex starts tomorrow, 5th May and finishes on Saturday 14th May. It's a little different to normal as it's a review of a documentation tool, rather than a library. We'll follow the normal procedure. Hopefully the review will also provide some experience for deciding how to deal with other tools in the future.

I am away during the review period (and my internet connection has gone wrong so even is email flakey), so I would like to 'pre-review' John's autoindex. First a confession - I have been nagging John for some time about a way of producing an index - (mainly of Boost.Math). I have been using the index for some libraries and found that it works well enough. It is not perfect (but then I suspect that no automatic index ever will be) - and lots of manually edited indexes are not that good either, and they cannot survive frequent updates to the text. Even an indifferent index is massively better than no index. (The only other tool I have used is to search the PDF version - if the index term has not been foreseen by the indexer, this remains a good tactic). But the results are certainly functional. (I can vouch for this because I have often used the index to find some information that I know is in my own docs but I am having a 'senior moment' about where ;-) It requires the author to specify the index terms - a skilled but tedious task requiring reading-the-mind-of-the-readers. The regex (I suppose it was inevitable written by Dr Regex himself) mechanism works well enough to deal with the many variants, plurals etc. Writing the regex expressions can be a bit tedious, but it works. The main criticism I have is that it tends to produce too many (often duplicate) index entries. But this is partly due to my lack of skill and patience in refining the index terms file. So I am strongly in favour of immediate acceptance of this tool, and of adoption by all authors using the QuickBook/Doxygen tool chain. I believe we need to do very much better on Boost libraries' documentation and this is an important tool to achieve that. Paul --- Paul A. Bristow, Prizet Farmhouse, Kendal LA8 8AB UK +44 1539 561830 07714330204 pbristow@hetp.u-net.com

Hi, This is not yet a review. On 11-5-2011 11:13, John Maddock wrote:

Just a reminder to everyone that this review is ongoing.... I realize there's a certain amount of "review fatigue" going on at present (!) - but some more reviews would be very welcome - details below.

Review - fatigue, might be. This tool is especially useful for Boost Library writers (more than for users). Though QuickBook or BoostBook can be used to document non-boost libraries, I doubt if that is done widely. I know at least one case. So, though this tool is really promising, this might explain the low volume of reviews or even questions or discussions on this list. (deliberately left this:)

About AutoIndex ===============

The documentation describes it as "a tool for taking the grunt work out of indexing a Boostbook/Docbook document that describes C/C++ code".

The documentation is online at:

http://svn.boost.org/svn/boost/sandbox/tools/auto_index/doc/html/index.html

AutoIndex can be downloaded from the vault:

http://www.boostpro.com/vault/index.php?action=downloadfile&filename=auto_index-0.9.zip

The download also contains a pdf of the documentation at 'doc/auto_index.pdf', and a copy of the html documentation at 'doc/html/index.html'.

I'm a Boost Library writer currently writing the doc's, so it is as if this tool is made for me and I feel obliged to write a review. I hope to finish it on time, but realize I'm a little late again and there is no much room for discussion (if necessary). Let me start with my review activities to write that I really like all produced indexes. I'll list them here explicitly: <http://www.boost.org/doc/libs/1_46_1/libs/spirit/doc/html/spirit/qi/s04.html> <http://www.boost.org/doc/libs/1_46_1/libs/spirit/doc/html/spirit/karma/s05.html> <http://www.boost.org/doc/libs/1_46_1/libs/type_traits/doc/html/index/s14.html> <http://www.boost.org/doc/libs/1_46_1/libs/math/doc/sf_and_dist/html/index/s16.html> I wish every Boost library had such an index. I also wish that Boost.Geometry will have such an index. So I will try to build one, and doing that, also finish my review. To be continued. Regards, Barend

Several questions: - Why Boost Program Options is not used? It is mimicked here

In the beginning I didn't think there were going to be enough command line options to require it..... and then they grew somewhat...

- If I see this: "if(internal_indexes == false)" I normally ask people why not write "if(! internal_indexes)" so I ask that here as well

Taste I guess, I generally prefer the explicit comparison <shug> I guess.

- Why is it indented at 3 spaces instead of the normal 4 as done e.g. in Boost.Math?

All my stuff uses 3 spaces, always has. Hangover from the influence Borland's OWL (anyone remember that?).

- Why Boost String Algorithms are not used and a function "make_upper_key" is created

Good question, will look into it.

- A copy of TinyXML is used here. Boost.PropertyTree uses RapidXML. It is really time to have a standard XML parser within Boost. I see from the copyright notes that this dates from 2002 so this is probably continuing existing Boost practice, so you might skip this comment

Amen to an official Boost XML parser! Actually the TinyXML version used has been modified for this particular use case...

- The method process_node is quite long, might be splitted (the whole block below comment "Search content for items" is an obvious candidate) - The method check_index_type_and_placement contains much code duplication especially in the exception messages

Overall I was not impressed by the implementation, but because it is a tool and not a library, I think lower code quality is acceptable.

Nod. It's currently typical of what happens when you don't know where the destination is when you start :-( There's a lot of room for refactoring/tidying up once it's clearer where the tool is headed, and what features are needed.

...

- What is your evaluation of the documentation?

It is good but not perfect. I've several notes about this, in addition to the notes that are made earlier by Edward.

About the name: the tool is called AutoIndex. The executable is called auto_index. We have to use it in the Jamfile by auto-index. The doc confuses this a bit by stating "full-path-of-executable-auto-index.exe" and also mentioning autoindex.exe (tutorial/configure)

Nod. I got a bit skitzophenic about the name. Will fix.

About quickbook-define: the semi-colon in the doc (tutorial/configure) should probably not be there. Besides that, it does not work for me (I get this error: "error: Invalid property '<quickbook-define>': No value specified for feature 'quickbook-define'.") So probably there is a term (auto-index?) missing there. Anyway, omitting it let the whole thing still work.

In the Script File (.idx) Reference, the reflex example is not explained and it is probably a duplicate of the example "\<myclass\w*\>"

On the same page, the sentence "If this third section selection field ... " is unclear - it is the second regex, the third field, "selection field" is mentioned here for the first time.

Same page, "functions, classs, macros " typo in "classs" (there is a myclasss where this is intentional)

The !debug regular-expression should IMO write the "regular-expression" in italics or something else to indicate it should be replaced

Will add to the fix list.

I don't understand this: "If you are writing a Quickbook document with Doxygen reference documentation, the position of a [xinclude autodoc.xml] line in the Quickbook file determines the location of the Doxygen references section" . It probably refers to an autodoc.xml created by a Boost XSLT tool (I'm not sure). That should be explained. I didn't dive into this and didn't ask questions on the list about this, it might be that it is generated by something else or a Doxygen option or that I just don't understand.

IMO It doesn't belong there - it was added by a pre-review reviewer.

...

- What is your evaluation of the potential usefulness of AutoIndex?

Very useful for people writing BoostBook or QuickBook, so the majority of the Boost library authors and some others.

...

- Did you try to use it? With what compiler? Did you have any problems?

Yes. I first tried it by calling the Jamfile. Then I noticed some compiler errors. Because I now know that bjam gets updated regularly, I called the booststrap batchfile. Then I tried again and everything builds fine.

In using and finetuning the index, I noticed that it indexes too much. That can be helped by adding them to excluded terms. Further, I was not able to exclude a term from a section. Discussions about this are still going on on this list.

In general, I do like the produced index, at least for Spirit, Math, TypeTraits. It provides much added value for library users. Also the samples (as discussed on this list) being indexed is useful (I only want to turn off some terms of them).

For Boost.Geometry, there is some added value. To really judge it, I would have to finetune it more. Currently it contains too much entries, as discussed. But (as not yet discussed) there are also missing entries. The term "reverse" contains only entries from the examples. The "reverse" function itself is not there. If I remove the reverse-examples, the whole term "reverse" is missing from the index. Don't know yet why.

Will investigate.

...

- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

I compiled it, configured it, used it and debugged it so yes, it was a kind of in-depth study. I think I spent more than 8 hours on it. Well, that was not only for this review alone, but to evaluate if it is useful for the Boost.Geometry documentation.

One of the issues with a tool like this is that it was developed with certain documentation in mind. Basically it needs to be tested/used by other docs writers to find out what's missing or needs to change for maximum versatility. So your feedback is most welcome.

...

I think it should be made clear that if accepted, there won't be any requirement to use AutoIndex it.

I think this freedom is essential. Though I tried it and voted Yes, I still don't know if we will use it in practice. I've to finetune it more and some things has to be solved, then we can judge the results.

Finally, I want to thank John Maddock for developing this useful tool and for help on the list, and Daniel James for managing the review.

And thank you very much for the in-depth review, very much appreciated! Regards, John.

...

...

About the name: the tool is called AutoIndex. The executable is called auto_index. We have to use it in the Jamfile by auto-index. The doc confuses this a bit by stating "full-path-of-executable-auto-index.exe" and also mentioning autoindex.exe (tutorial/configure)

Nod. I got a bit skitzophenic about the name. Will fix.

Small point...

I would be better to call Boost tools with some Boost prefix "namespace" like boost_auto_index.exe or at leaset bauto_index similary to bjam or to bcp.

Rationale, auto_index /autoindex is something too general that may collide with other programs for example all GNU autotools start with auto - autoconf, automake, autoXXX. So one day there may be autoindex.

I'd suggest to use "boost" namespace like name for executables as well so it would not collide with other installed programs.

It is especially critical for single root OS like Linux and other Unixes where all programs go to /usr/bin and then collisions can easily happen.

Good points all, John.

Hi John, On 16-5-2011 19:41, John Maddock wrote:

...

For Boost.Geometry, there is some added value. To really judge it, I would have to finetune it more. Currently it contains too much entries, as discussed. But (as not yet discussed) there are also missing entries. The term "reverse" contains only entries from the examples. The "reverse" function itself is not there. If I remove the reverse-examples, the whole term "reverse" is missing from the index. Don't know yet why.

I investigated this - the regular expressions used for searching for functions in docs were too strict - basically they were expecting your function declarations to terminate in a ";" and yours don't.

I've updated the code (and docs to match) in SVN to adjust this, and it's working as expected for the Geometry docs now - also finding many more functions in the Boost.Math docs!

Great! Thanks for this. Eehm, I know it is not related to AutoIndex, and it is probably my or a few people's problem. I've googled, at least some other (Boost) people have had this as well. I get this error while building: /*error: Attempt to redeclare already existing project id '/boost/exception' location '/_svn/boost/trunk/libs/exception/build' */ I've had this also when building Boost.Geometry. Then I could solve it by removing Jamroot, or adding one. It is only on Microsoft, not on Linux or MinGW. The error always says "exception", strange thing... Do other people using MSVC have (had) this problem? Strangely enough it sometimes occurs, so last time I could build autoindex, now I couldn't do it (yet). Anyway, I will try soon again, probably using Linux, and let you know. Regards, Barend