Added CMakeLists.txt and fixed C++17 compilation on Windows #10
Conversation
prometey1982
commented
Jan 2, 2025
prometey1982
commented
- Removed unused headers
- Added CMakeLists.txt for including into projects with CMake
- Fixed compilation for modern C++
|
@bfoz can look into? |
| #include "intelhex.h" | ||
|
|
||
| #include <fstream> |
There was a problem hiding this comment.
I prefer to order includes with system (or std) includes before local includes
There was a problem hiding this comment.
In such case you can get into situation when .h file uses something from implementation but not includes it.
| @@ -350,7 +348,7 @@ namespace intelhex | |||
|
|
|||
| while( (s.get() == ':') && s.good() ) | |||
| { | |||
| getline(s, line); // Read the whole line | |||
| std::getline(s, line); // Read the whole line | |||
There was a problem hiding this comment.
getline is inside std namespace
https://en.cppreference.com/w/cpp/string/basic_string/getline
|
Sorry for the delay. I don't use CMake, so I have no way of validating that part, or any way of maintaining it. Does CMake have a backwards-compatibility mode for using Makefiles? The header changes look simple enough. Are those changes Windows-specific? Which parts are C++17 specific? |
|
I think that previously getline was not inside std namespace or something like this. Maybe there was some macros. When I took your library for my purposes I found that it has some Linux specific includes and doesn't compile with Visual Studio C++ 2022 with enabled C++17. This is way I named this PR as fix for C++17 on Windows. |
