cmake: cpptoml allow usage of installed dependency

[davidwed]

No description provided.

[codecov]

Codecov Report

Merging #40 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #40   +/-   ##
=======================================
  Coverage   87.08%   87.08%           
=======================================
  Files           4        4           
  Lines         480      480           
=======================================
  Hits          418      418           
  Misses         62       62

Impacted Files	Coverage Δ
include/spdlog_setup/details/conf_impl.h	89% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb6bfe3...5f33831. Read the comment docs.

[codecov]

Codecov Report

Merging #40 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #40   +/-   ##
=======================================
  Coverage   87.08%   87.08%           
=======================================
  Files           4        4           
  Lines         480      480           
=======================================
  Hits          418      418           
  Misses         62       62

Impacted Files	Coverage Δ
include/spdlog_setup/details/conf_impl.h	89% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb6bfe3...ee2b91d. Read the comment docs.

[guangie88]

Hi, thanks again for the PR. I have seen the code changes, but after some consideration, I have reservations to accept the PR.

While this change would give the flexibility of allowing cpptoml to be externally included, therefore possibly allowing the header file to be shared among other dependencies in a project, this however is very likely a rare case. When compared to spdlog, spdlog was specially placed into a submodule, because spdlog is the main library to support, and this repo exists to support/augment spdlog and support a range of versions of spdlog. cpptoml on the other hand, doesn't fall within this consideration.

Additionally, I have made the presumption that the user is likely to have tried out spdlog already but realized that it doesn't come with a config based set-up. He/she would later somehow discover this repo and think of this repo as an add-on to spdlog. As such, embedding cpptoml within spdlog_setup would allow user to simply download the source zip, and just copy out the include/spdlog_setup, without having to deal with submodules or CMake installation.

I hope the above are reasonable explanations to keep the way cpptoml as it currently is.

[davidwed]

With the cmake option "SPDLOG_SETUP_CPPTOML_EXTERNAL" you can now decide if you want to use the internal cpptoml or an external one.

The default is false, so you can still download the zip, unpack it and use it exactly like before the change.

[guangie88]

Thanks for spending the effort for this, it sounds good, and I am generally in favor as long as it is an opt-in, which is what you have described.

I will take a closer look then, but meanwhile will give some small comments here and there if you don't mind.

[guangie88]

/test

[guangie88]

LGTM, but would need you to run ./run-clang-format.sh, getting lint error from:
include/spdlog_setup/details/conf_impl.h

[guangie88]

/test