Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for the fmt library #185

Open
MCWertGaming opened this issue Aug 23, 2022 · 5 comments
Open

add support for the fmt library #185

MCWertGaming opened this issue Aug 23, 2022 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@MCWertGaming
Copy link
Collaborator

We should add support for the fmt library since it advertises a much better performance than iostream.
https://github.com/fmtlib/fmt

@MCWertGaming MCWertGaming added the enhancement New feature or request label Aug 23, 2022
@MCWertGaming MCWertGaming added this to the V2.0.0 milestone Aug 23, 2022
@flagarde
Copy link
Collaborator

flagarde commented Aug 24, 2022

Do you know why you are using cmake 3.10 as minimum...

With 3.11 we could have 'https://cmake.org/cmake/help/latest/module/FetchContent.html'... It would allow to supress doctest from the repo and only download it at configure time

With cmake 3.14 we would have an easier syntax for it and we would have the possibility to use https://github.com/cpm-cmake/CPM.cmake

The benefits I see to use at least FetchContent is that you can easily change the version of your dependencies and make some checks... Then by just a change of variable change the version of the dependency... And the livrary is not directly included on the repo... Doctest is wonderful but you are using a old version on cpp_terminal and moving need to change all the doctest.h

With fetchcontent it should be possible to just change a 'set(USE_DOCTEST_VERSION "...")'

The downloading of the dependecies could even be skip if the user have the dependencies already installed on his computer.

Personally I love FetchContent and thank CMake people to have included this features 🙏 into CMake

Look the magic :

cmake_minimum_required(VERSION 3.14 FATAL_ERROR) 
  
 project(CPMExampleDoctest) 
  
 # ---- Dependencies ---- 
  
 include(../../cmake/CPM.cmake) 
  
 CPMAddPackage("gh:cpm-cmake/[email protected]") 
 CPMAddPackage("gh:onqtam/doctest#2.4.5") 
  
 # ---- Create binary ---- 
  
 add_executable(CPMExampleDoctest main.cpp) 
 target_link_libraries(CPMExampleDoctest fibonacci doctest) 
 target_compile_features(CPMExampleDoctest PRIVATE cxx_std_17) 
  
 # ---- Enable testing ---- 
  
 enable_testing() 
 add_test(CPMExampleDoctest CPMExampleDoctest)

@MCWertGaming
Copy link
Collaborator Author

I wouldn't include fmt directly into cpp-terminal, just add support so people can add it themselves to their project. The thing with dependencies is that it has to be maintained and managed. Also some users might not want to use fmt for various reasons.

There is no exact reason why the cmake version is so low, but out of my personal experience I know that high versions will break the compilation on some systems like Debian, Ubuntu, all those distros with way to old software.

I'd like to move from doctest to google test as I have a lot of experience with it and really like it's syntax. For that I'll probably use git sub modules that are just fetches from cmake. While It's also a good practice to disable the unit testing dependencies when the project is used in another project and to use the locally installed version over the bundled (important for packaging, in this case it speeds up compile time).

@MCWertGaming
Copy link
Collaborator Author

But the cmake needs refactoring anyway

@flagarde
Copy link
Collaborator

Cmake 3.10 is for ubuntu 18 (2018). In ubuntu 20 (2020) it's cmake 3.16.

Yes, I know debian/Ubuntu 🙄😔...

@MCWertGaming
Copy link
Collaborator Author

Yeah, we can implement it only for a specific cmake version though. Otherwise as long as the CI is passing it should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: To do
Development

No branches or pull requests

2 participants