[sword-devel] cbible

[Isaac Dunham]

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