Fix incompatibility with JDK, code inconsistencies and upgrade to C++17 #120
Conversation
…ault enum memory representation
…Lib namespace to prevent global scope pollution
…t's fully support starting with Windows 8
…ShellLinkPath to use std::optional and std::filesystem::path
…IWinToastHandler into its own header Refactor out Windows specific includes into Platform.h, refactor out IWinToastHandler into its own header
…ADED to avoid JDK incompatibility issues due to JDK-8353140. Add nodiscard attributes, cleanup code further
|
Gonna fix the last remaining issues with this later today hopefully :) |
There was a problem hiding this comment.
This PR would be much easier to review and discuss if it was split into multiple PRs. GitHub makes it really hard to review individual commits on PRs.
I added a few general comments. std::wstring_view can probably be used a bit more, but again, in a separate PR.
| #include <Windows.h> | ||
| #include <wrl/event.h> | ||
| #include <windows.ui.notifications.h> | ||
| #include <roapi.h> | ||
|
|
||
| namespace WinToastLib { | ||
| using namespace Microsoft::WRL; | ||
| using namespace ABI::Windows::Data::Xml::Dom; | ||
| using namespace ABI::Windows::Foundation; | ||
| using namespace ABI::Windows::UI::Notifications; | ||
| using namespace Windows::Foundation; | ||
| } |
There was a problem hiding this comment.
Ideally, if one includes WinToast, they shouldn't need to include Windows libraries.
There was a problem hiding this comment.
The name of this file is too generic. Ideally, WinToast headers would be in a wintoast/ subdirectory. That would be possible if we make breaking changes anyway.
| file(GLOB_RECURSE WINTOASTLIB_HEADERS "${CMAKE_CURRENT_LIST_DIR}/include/*.h") | ||
| file(GLOB_RECURSE WINTOASTLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/src/*.cpp") |
There was a problem hiding this comment.
Globs for source files are discouraged. See CMake docs:
We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate.
| }; | ||
| } | ||
|
|
||
| #endif //IWINTOASTHANDLER_H No newline at end of file |
There was a problem hiding this comment.
All new files are missing final newlines.
2 participants
Right now this is a draft, but i started working on a 2.0.0 of this library.
Sadly due to the JDK bug, this has to be break the current ABI, so a major version bump is required.
Any suggestions/requests are appreciated, as of right now the progress looks like this:
I'll try to finish this as soon as possible :)