toml11 icon indicating copy to clipboard operation
toml11 copied to clipboard

Non optimal header location and inclusion

Open Naios opened this issue 4 years ago • 3 comments

Hey, first of all thanks for your library.

I've noticed that your directory structure could possibly causes inclusion issues because there is no toplevel (include) directory which seperates the inclusion path from the rest of the repository.

A better (and more established) way would be to move all your public headers into a directory like include/toml such that no one has to pass the whole project directory to the compiler as inclusion directory. This could possibly mess up other includes otherwise (and also it exposes the whole repo to the compiler).

You could also then reference your own headers through an absolut path:

#include <algorithm>
#include <toml/from.hpp>
#include <toml/result.hpp>
#include <toml/value.hpp>

Additionally the google C++ style guide recommends to include standard headers before the own ones in alphabethical order to profit from a possible inclusion order optimization: https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

Naios avatar Apr 17 '20 17:04 Naios

I see, I understand. You are right.

However, some projects use this repository via git submodule, and changing the include path would be a breaking change. So I'll solve this when I release version 4.

The inclusion order can be improved without any changes, so I will do that soon.

ToruNiina avatar May 11 '20 05:05 ToruNiina

Theoretically you could copy the headers into an include/toml11 directory while keeping the old files. The content of the old files then could be replaced by a #include <toml11/CURRENT_FILE.hpp while producing a deprecated inclusion #warning. Through that you would keep the semantic versioning (because nothing breaks) while also giving your users a notice for migration and to make it possible for everybody to use the new inclusion right away.

Naios avatar May 11 '20 06:05 Naios

That sounds nice. When we see the prospect of releasing version 4, I will add it.

I have several tasks for the next release, like std::u8string support and improvement of options while serialization, so it would take a while.

ToruNiina avatar May 11 '20 08:05 ToruNiina