From 02295f4106a9de2972c8101b20caecd309d70cb6 Mon Sep 17 00:00:00 2001 From: ptahmose Date: Sat, 6 Jul 2024 00:57:09 +0200 Subject: [PATCH] Refactor CI and enhance code robustness - Removed OpenCppCoverage and Codecov integration from cmake.yml, indicating a shift in coverage analysis strategy. - Added automated packaging of CZIrepair-binary for Windows Release builds to streamline distribution. - Deleted GitHub Pages deployment workflow (pages.yml), suggesting a change in documentation hosting strategy. - Enhanced C++ code documentation and made codebase improvements in `commandlineoptions.h` and `czirepair.cpp`, including adding documentation comments, making variables `const`, standardizing namespace usage, and introducing more detailed logging. - Improved error handling in `repairutilities.cpp` for better robustness and added documentation comments to `SubBlockDimensionInfoRepairInfo` struct. - Overhauled Unicode support in `utilities.cpp` for accurate UTF-8 and wide character string conversions, reflecting these changes in `utilities.h` function signatures. --- .github/workflows/cmake.yml | 25 -------------- .github/workflows/pages.yml | 46 -------------------------- Src/czirepair/commandlineoptions.h | 5 +-- Src/czirepair/czirepair.cpp | 53 +++++++++++++++++++----------- Src/czirepair/repairutilities.cpp | 14 ++++++-- Src/czirepair/repairutilities.h | 15 +++++++-- Src/czirepair/utilities.cpp | 38 +++++++++++++++++++-- Src/czirepair/utilities.h | 4 +-- 8 files changed, 99 insertions(+), 101 deletions(-) delete mode 100644 .github/workflows/pages.yml diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 8b6ddf9..e345791 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -69,31 +69,6 @@ jobs: # Use debug flag to show all exeucted tests run: ctest --debug -C ${{matrix.build}} - # # Coverage collection based on https://about.codecov.io/blog/how-to-set-up-codecov-with-c-plus-plus-and-github-actions/ - # - name: Prepare Coverage - # if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Debug') }} - # run: | - # choco install OpenCppCoverage -y --no-progress - # echo "C:\Program Files\OpenCppCoverage" >> "$env:GITHUB_PATH" - - # - name: Get Coverage - # if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Debug') }} - # working-directory: ${{github.workspace}}/build/Src/libCZI_UnitTests/${{matrix.build}} - # shell: cmd - # run: OpenCppCoverage.exe --export_type cobertura:${{github.workspace}}\coverage.xml --config_file "${{github.workspace}}\opencppcoverage.txt" -- libCZI_UnitTests.exe - - # - name: Upload Coverage - # uses: codecov/codecov-action@v4 - # if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Debug') }} - # with: - # files: ./coverage.xml - # fail_ci_if_error: true - # verbose: true - # # Only one flag to be safe with - # # https://docs.codecov.com/docs/flags#one-to-one-relationship-of-flags-to-uploads - # flags: ${{matrix.OS}} - # token: ${{ secrets.CODECOV_TOKEN }} - # gather the CZIrepair-binary and put it into a folder - it will be zipped with the upload step - name: Package if: ${{ (matrix.OS == 'windows-latest') && ( matrix.build == 'Release') }} diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml deleted file mode 100644 index 952daa2..0000000 --- a/.github/workflows/pages.yml +++ /dev/null @@ -1,46 +0,0 @@ -# Simple workflow for deploying static content to GitHub Pages -name: Pages - -on: - # Runs on pushes targeting the default branch - push: - branches: ["main"] - -# Sets permissions of the GITHUB_TOKEN to allow deployment to GitHub Pages -permissions: - contents: read - pages: write - id-token: write - -# Allow one concurrent deployment -concurrency: - group: "pages" - cancel-in-progress: true - -jobs: - # Single deploy job since we're just deploying - deploy: - environment: - name: github-pages - url: ${{ steps.deployment.outputs.page_url }} - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Install Doxygen - run: sudo apt-get install doxygen -y - shell: bash - - name: Generate Doxygen Documentation - working-directory: ${{github.workspace}}/Src - run: doxygen Doxyfile - shell: bash - - name: Setup Pages - uses: actions/configure-pages@v2 - - name: Upload artifact - uses: actions/upload-pages-artifact@v1 - with: - # Upload entire repository - path: "${{github.workspace}}/Src/Build/html/" - - name: Deploy to GitHub Pages - id: deployment - uses: actions/deploy-pages@v1 diff --git a/Src/czirepair/commandlineoptions.h b/Src/czirepair/commandlineoptions.h index b4fddcf..80d08ca 100644 --- a/Src/czirepair/commandlineoptions.h +++ b/Src/czirepair/commandlineoptions.h @@ -2,13 +2,14 @@ #include +/// Values that represent the command - the operation this program should perform. enum class Command { Invalid = 0, - DryRun, + DryRun, ///< An enum constant representing the "dry run" option - i.e. just determine what would be done. - Patch, + Patch, ///< An enum constant representing the "patch option" - i.e. actually patch the file if necessary. }; enum class Verbosity diff --git a/Src/czirepair/czirepair.cpp b/Src/czirepair/czirepair.cpp index a4afc9a..1f89c99 100644 --- a/Src/czirepair/czirepair.cpp +++ b/Src/czirepair/czirepair.cpp @@ -15,19 +15,19 @@ namespace { void DryRun(const CommandLineOptions& options) { - shared_ptr stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str()); - auto reader = libCZI::CreateCZIReader(); + const shared_ptr stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str()); + const auto reader = libCZI::CreateCZIReader(); reader->Open(stream); vector repair_info = RepairUtilities::GetRepairInfo(reader.get()); if (repair_info.empty()) { - cout << "No repair needed." << endl; + cout << "No repair needed." << std::endl; } else { - cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << endl; + cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << std::endl; if (options.IsVerbosityGreaterOrEqual(Verbosity::Verbose)) { @@ -38,7 +38,7 @@ namespace cout << "SubBlockIndex: " << info.sub_block_index << " -> size in 'dimension_info': " << sub_block_info.physicalSize.w << "x" << sub_block_info.physicalSize.h << ", JPGXR: " << (info.IsFixedSizeXValid() ? info.fixed_size_x : sub_block_info.physicalSize.w) << "x" << - (info.IsFixedSizeYValid() ? info.fixed_size_y : sub_block_info.physicalSize.h) << endl; + (info.IsFixedSizeYValid() ? info.fixed_size_y : sub_block_info.physicalSize.h) << std::endl; } } } @@ -49,8 +49,8 @@ namespace vector repair_info; { - shared_ptr stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str()); - auto reader = libCZI::CreateCZIReader(); + const shared_ptr stream = libCZI::CreateStreamFromFile(options.GetCZIFilename().c_str()); + const auto reader = libCZI::CreateCZIReader(); reader->Open(stream); repair_info = RepairUtilities::GetRepairInfo(reader.get()); @@ -62,34 +62,33 @@ namespace } else { - cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << endl; + cout << "Found discrepancies with " << repair_info.size() << " sub-block(s)." << std::endl; if (options.IsVerbosityGreaterOrEqual(Verbosity::Verbose)) { for (const auto& info : repair_info) { cout << "SubBlockIndex: " << info.sub_block_index << " -> size in 'dimension_info': " << - info.fixed_size_x << "x" << info.fixed_size_y << endl; - cout << endl; + info.fixed_size_x << "x" << info.fixed_size_y << std::endl; + cout << std::endl; } } } } - cout << "Now opening the file in read-write-mode and will patch the file." << endl; + cout << "Now opening the file in read-write-mode and will patch the file." << std::endl; shared_ptr input_output_stream = libCZI::CreateInputOutputStreamForFile(options.GetCZIFilename().c_str()); RepairUtilities::PatchSubBlockDimensionInfoInSubBlockDirectory(input_output_stream.get(), repair_info); - cout << "Patched the subblock-directory." << endl; + cout << "Patched the subblock-directory." << std::endl; RepairUtilities::PatchSubBlocks(input_output_stream.get(), repair_info); - cout << "Patched the subblocks." << endl; + cout << "Patched the subblocks." << std::endl; input_output_stream.reset(); } } - int main(int argc, char** argv) { CommandLineOptions options; @@ -107,11 +106,28 @@ int main(int argc, char** argv) } else if (commandline_parsing_result == CommandLineOptions::ParseResult::Exit) { - return 0; + return EXIT_SUCCESS; } - //std::cout << "Command: " << static_cast(options.GetCommand()) << std::endl; - //std::cout << "CZI filename: " << options.GetCZIFilename() << std::endl; + if (options.IsVerbosityGreaterOrEqual(Verbosity::Verbose)) + { + const char* command_text; + switch (options.GetCommand()) + { + case Command::DryRun: + command_text = "DryRun"; + break; + case Command::Patch: + command_text = "Patch"; + break; + default: + command_text = "Invalid"; + break; + } + + std::cout << "Command: " << command_text << std::endl; + std::cout << "Filename: " << Utilities::convertToUtf8(options.GetCZIFilename()) << std::endl << std::endl; + } if (options.GetCommand() == Command::DryRun) { @@ -122,6 +138,5 @@ int main(int argc, char** argv) Patch(options); } - - return 0; + return EXIT_SUCCESS; } diff --git a/Src/czirepair/repairutilities.cpp b/Src/czirepair/repairutilities.cpp index 3fd88de..67ea107 100644 --- a/Src/czirepair/repairutilities.cpp +++ b/Src/czirepair/repairutilities.cpp @@ -9,16 +9,25 @@ std::vector RepairUtilities::G { std::vector result; + // go through all sub-blocks reader->EnumerateSubBlocks( [&](int index, const SubBlockInfo& subblock_info)->bool { + // we are only interested in JPGXR-compressed sub-blocks if (subblock_info.GetCompressionMode() == CompressionMode::JpgXr) { - auto sub_block = reader->ReadSubBlock(index); + // read the sub-block + const auto sub_block = reader->ReadSubBlock(index); + // determine the width and height of the JPGXR-compressed bitmap uint32_t width_from_jpgxr, height_from_jpgxr; - sub_block->TryGetWidthAndHeightOfJpgxrCompressedBitmap(width_from_jpgxr, height_from_jpgxr); + if (!sub_block->TryGetWidthAndHeightOfJpgxrCompressedBitmap(width_from_jpgxr, height_from_jpgxr)) + { + throw runtime_error("Failed to determine the width and height of the JPGXR-compressed bitmap."); + } + // Now, compare the width and height of the JPGXR-compressed bitmap with the width and height in the dimension-info. + // If they differ, we need to fix the dimension-info - add the sub-block to the list of sub-blocks that need to be fixed. SubBlockDimensionInfoRepairInfo repair_info; repair_info.sub_block_index = index; if (subblock_info.physicalSize.w != width_from_jpgxr) @@ -31,6 +40,7 @@ std::vector RepairUtilities::G repair_info.fixed_size_y = height_from_jpgxr; } + // If the width or height of the sub-block should be fixed, add the sub-block to the list of sub-blocks that need to be fixed. if (repair_info.IsFixedSizeXValid() || repair_info.IsFixedSizeYValid()) { result.push_back(repair_info); diff --git a/Src/czirepair/repairutilities.h b/Src/czirepair/repairutilities.h index 2da4423..ec57ebb 100644 --- a/Src/czirepair/repairutilities.h +++ b/Src/czirepair/repairutilities.h @@ -7,13 +7,22 @@ class RepairUtilities { public: + /// This structure gather information about fixes that need to be applied to a sub-block's dimension-info. struct SubBlockDimensionInfoRepairInfo { - int sub_block_index{ -1 }; - std::uint32_t fixed_size_x{ std::numeric_limits::max() }; - std::uint32_t fixed_size_y{ std::numeric_limits::max() }; + int sub_block_index{ -1 }; ///< This index of the sub block. + std::uint32_t fixed_size_x{ std::numeric_limits::max() }; ///< The width the sub-block should have (in the dimension-info). + std::uint32_t fixed_size_y{ std::numeric_limits::max() }; ///< The height the sub-block should have (in the dimension-info). + + /// Query if the width of the sub-block should be fixed (and the property fixed_size_x is valid). + /// + /// \returns True if the property fixed_size_x is valid, false if not. bool IsFixedSizeXValid() const { return fixed_size_x != std::numeric_limits::max(); } + + /// Query if the width of the sub-block should be fixed (and the property fixed_size_y is valid). + /// + /// \returns True if the property fixed_size_y is valid, false if not. bool IsFixedSizeYValid() const { return fixed_size_y != std::numeric_limits::max(); } }; diff --git a/Src/czirepair/utilities.cpp b/Src/czirepair/utilities.cpp index 5caa953..54d259b 100644 --- a/Src/czirepair/utilities.cpp +++ b/Src/czirepair/utilities.cpp @@ -10,18 +10,52 @@ using namespace std; -std::string Utilities::convertToUtf8(const std::wstring& str) +std::string Utilities::convertToUtf8(const std::wstring& wide_str) { +#if defined(WIN32ENV) + int byte_count = WideCharToMultiByte(CP_UTF8, 0, wide_str.c_str(), -1, NULL, 0, NULL, NULL); + if (byte_count == 0) + { + // Handle error (e.g., invalid wstring) + return ""; // Empty string on error + } + + // Allocate memory for the UTF-8 string (including null terminator) + std::string utf8_str(byte_count, '\0'); + + // Perform the actual conversion + WideCharToMultiByte(CP_UTF8, 0, wide_str.c_str(), -1, &utf8_str[0], byte_count, NULL, NULL); + + return utf8_str; +#else std::wstring_convert> utf8_conv; std::string conv = utf8_conv.to_bytes(str); return conv; +#endif } -std::wstring Utilities::convertUtf8ToUCS2(const std::string& str) +std::wstring Utilities::convertUtf8ToUCS2(const std::string& utf8_str) { +#if defined(WIN32ENV) + int wide_char_count = MultiByteToWideChar(CP_UTF8, 0, utf8_str.c_str(), -1, NULL, 0); + if (wide_char_count == 0) + { + // Handle error (e.g., invalid UTF-8) + return L""; // Empty wstring on error + } + + // Allocate memory for the wide character string + std::wstring wide_str(wide_char_count, L'\0'); + + // Perform the actual conversion + MultiByteToWideChar(CP_UTF8, 0, utf8_str.c_str(), -1, &wide_str[0], wide_char_count); + + return wide_str; +#else std::wstring_convert> utf8conv; std::wstring conv = utf8conv.from_bytes(str); return conv; +#endif } #if defined(WIN32ENV) diff --git a/Src/czirepair/utilities.h b/Src/czirepair/utilities.h index a6989a3..4157c97 100644 --- a/Src/czirepair/utilities.h +++ b/Src/czirepair/utilities.h @@ -7,8 +7,8 @@ class Utilities { public: - static std::string convertToUtf8(const std::wstring& str); - static std::wstring convertUtf8ToUCS2(const std::string& str); + static std::string convertToUtf8(const std::wstring& wide_str); + static std::wstring convertUtf8ToUCS2(const std::string& utf8_str); }; #if defined(WIN32ENV)