On Sat, Aug 01, 2015 at 09:07:41PM -0400, Tim Hawes wrote: > > Isaac Dunham writes: > > > While I'm not the one who reported the issue, I had a couple easily-fixed > > issues with headers (readline needed FILE, and a missing <iostream>). > > Patch attached. > > That patch just moves the readline includes to the middle of the include > list. I am trying to follow the Google C++ coding style as closely as > possible, and that style says that C header includes happen before C++ > includes (not to mention that re-ordering the readline includes does > nothing).
On the system I used, readline fails to pull in the headers that define FILE. Thus, they *must* follow stdio.h or a header that includes it. iostream fits this requirement, so it does not do nothing; but if you'd rather use stdio.h instead, it's your project. For what it's worth, I'd be surprised if the style guide didn't say "libraries after system headers". > It also adds iostream to Options.hpp which does not use > iostream at all. Ah, Options.hpp is public, rather than a header for whatever Options.cpp needs. Options.cpp requires iostream for std::cerr. > The only missing include, which this patch does not > address would be string in Options.hpp. While the gcc and clang > compilers do not care, it does violate Google style. I'll be sure to > correct it. > > As much as I appreciate code patches, I would much prefer pull requests > on GitHub, since that is where I am hosting this project. OK, noted. Thank you and God bless. Isaac Dunham _______________________________________________ sword-devel mailing list: sword-devel@crosswire.org http://www.crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page