-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 John, please find attached a patch plus a short summary of notes. More extensive comments: Jamfile - ------- While auto-index is still not a part of boost, I think you should check for the environmental variable BOOST_ROOT so that users can checkout auto-index and use it outside of their boost-trunk source tree. I see that you had: local boost = [ modules.peek : BOOST ] ; I've never seen the above done before to locate boost. I usually use: import os ; local BOOST_ROOT = [ os.environ BOOST_ROOT ] ; I'm sure volodya could tell us which is more portable. Other problems with the Jamfile; the targets for rgeex, filesystem and system are bad for GCC and clang (copious linker errors). Instead of: $(boost)/libs/regex/build//boost_regex $(boost)/libs/filesystem/build//boost_filesystem $(boost)/libs/system/build//boost_system you should have: $(boost)//regex $(boost)//filesystem $(boost)//system This leads to a successful build for me with gcc and clang. This is how the wave tool specifies libraries to be linked, so I'm assuming this will work fine on MSVC, etc. Source Code - ----------- The long initializer for the 257-element boost::array<> at line 168 in auto_index.cpp is painful to look at, and raises warnings with clang. I suggest reformatting it. A few other places in the code have painfully long lines (see: attached patch). Why aren't you using program_options for the command line argument parsing? Is there a particular rationale here, or is this something you didn't consider? All in all, the code does what it's supposed to (we've experienced no bugs/problems with this tool in the Spirit docs yet that I know of). However, I do have reservations about the design of the tool, though I suspect these reservations may be entirely stylistic. I have similiar reservations about other Boost tools, so perhaps I am biased. I feel like this code, while usable, is not particularly extensible. It feels like (no offense) a typical C application; copious use of source-file local global-namespaced functions and globals instead of tying said functions and globals together into classes which could be reused and hard-coded defaults preferred to more generic, configurable code. This is intended as a tool, and not a library, so this is understandable, moreso because the tool does exactly what it's supposed to do. On the other hand, I'd rather see a tool that is a library. An excellent example of this design model is the Clang project. Clang is a C language compiler, and at first glance it would appear to be "application" software. However, it is, in fact, "library" software; Clang is, first and foremost, a set of libraries which are designed to be reusable and extensible. The Clang driver (aka the "tool") is just an application that makes use of the Clang libraries. Finally, as a Spirit developer, I feel the need to make a shameless plug for my library :x. I wonder if some of the regex code used in auto_index (in particular, the stuff in file_scanning.cpp) wouldn't be better implemented with Spirit or Xpressive. I see a lot of regexs hard coded in strings, which implies a runtime cost to parse those strings... that cost could be possibly be avoided by rewriting that code as expression templates with Spirit or Xpressive. OTOH, that's probably more trouble than it's worth. Finally, another optimization thing - I've noticed for the Spirit docs (193 terms), indexing can take a while. Nothing too excessive, but certainly a noticable amount of time. As auto-index seems to currently work as intended, and there appear to be no bugs in it, perhaps investigating some optimizations to increase the speed of indexing would be prudent? I hope this feedback is helpful John. This is a great tool, and it has excellent and extensive documentation. I would hope that it would ship with Boost 1.47. Sincerely, - -- Bryce Lelbach aka wash boost-spirit.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEARECAAYFAk05kcMACgkQ9cB/V3/s9ExzNgCghvdSB9TGbREtPqPf8Dldv/xp 8OoAn0ok5u76QHhiSVyO6H4TDvHrlz8l =gBBq -----END PGP SIGNATURE-----