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.