Repository navigation
fix(plugin-catalog): preserve Unicode paths in failure diagnostics - #226
Draft
reliable-ly0411 wants to merge 4 commits into
Draft
reliable-ly0411 wants to merge 4 commits into
reliable-ly0411 wants to merge 4 commits into
Conversation
facontidavide
force-pushed
the
fix/plugin-catalog-unicode-errors
branch
from
November 13, 2026 23:26
52cdf32 to
1444051
Compare
Resolve the Unreleased changelog conflict by retaining both the plugin catalog fix and upstream pre-1.0 changes. Preserve maintainer-rebased source and regression test unchanged.
…e cannot be stored The test created its "資料-🚀" directory with the throwing overload of create_directories, so on a filesystem that rejects supplementary-plane characters or a non-Unicode encoding it failed in setup although the catalog behaved correctly. It now uses the error_code overload and skips for an illegal byte sequence or invalid argument; any other I/O error still fails. Found by a Codex review of this pull request. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(plugin-catalog): preserve Unicode paths in failure diagnostics
Summary
detail::pathForLegacyAccessorfor the suffix check and six error-formatting sites in the plugin catalog. On Windows this uses UTF-8 rather than the active code page; Unix behavior stays unchanged.Why
MSVC
std::filesystem::path::string()can throwstd::system_error(1113 / ERROR_NO_UNICODE_TRANSLATION) while formatting the intended loader error. The API consequently throws instead of returning its Expected error or scan diagnostic. The SDK already has a UTF-8 path conversion helper used by its loader; this patch reuses it rather than adding a second encoder or swallowing exceptions.Evidence
A focused Windows MSVC 19.44 / ACP 1252 A/B experiment compiled actual SDK 0.39.0 catalog source: all six original cases threw code 1113; all six patched cases passed.
https://github.com/reliable-ly0411/PlotJuggler-zh-CN/actions/runs/42784741091
Current main 9fc42b1 has the identical original catalog source SHA256 as 0.39.0. The patch applies cleanly. The final GTest regression extracted with its actual fixture passes locally on Linux; clang-format 22.1.0 and diff whitespace checks pass.
Follow-up Linux validation
Tested commit:
52cdf329adf6af3513e190fe0f8b86f19f17b051(the current PR head), with no tracked-file changes.pre-commit run --all-files: all applicable hooks passed, including clang-format 22.1.0.git diff --checkpassed../build.sh --debug: dependencies and configuration completed with the existing Conan/CMake toolchain. An initial two-job build had a compiler process killed by the environment. Resuming withcmake --build build/debug_asan -j1completed the full build successfully, without code changes. The cache hasCMAKE_BUILD_TYPE=Debug,PJ_ENABLE_SANITIZERS=ON, andPJ_ASSERT_THROWS=ON../test.sh: 5/105 CTest entries passed; 100 failed with LeakSanitizer fatal errors in this sandbox/ptrace environment. The new integrated Unicode regression itself passed before the process-exit leak check. This is not a passing default sanitizer run.ASAN_OPTIONS=detect_leaks=0 ./test.sh: 105/105 CTest entries passed, including the complete 36-test plugin catalog executable. AddressSanitizer instrumentation remains enabled; leak detection was explicitly disabled and is not validated.Remaining validation
LeakSanitizer needs a compatible host. The new integrated GTest and the full SDK/application suites still need Windows validation. The earlier Windows A/B covered the catalog implementation, not those complete suites. The original application's
RuntimeCatalogLanguageTestis not claimed fixed until rebuilt and tested against an official fixed SDK release.No production SDK pins or release artifacts were changed. This remains a draft; the repository's platform CI skips drafts, so no full CI pass is claimed.
Fixes #225.