-
-
Notifications
You must be signed in to change notification settings - Fork 7
fix: correct JSON string escaping, improve numeric value assignment, and add tests #10
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
fix: correct JSON string escaping, improve numeric value assignment, and add tests #10
Conversation
- Added option to build a local test executable for value assignments. - Enhanced JSON string formatting by escaping special characters. - Updated README with build instructions and prerequisites. - Modified .gitignore to include .vscode directory. - Added file read/write tests in format_test.cpp. - Refactored value assignment logic to reduce code duplication. Signed-off-by: YongDo-Hyun <[email protected]>
|
Thanks for review @Trial97 build and test results in here thanks. samet@192: /src/github/orgs/Project-Tick/libnbtplusplus$ cmake -S . -B build -G Ninja 100% tests passed, 0 tests failed out of 7 Total Test time (real) = 0.03 sec |
|
why is this pr touching half the code base with code style breaking formatting? I can't even tell what code is fixing the json anymore. |
Signed-off-by: YongDo-Hyun <[email protected]>
… in json_formatter; add make_numeric_tag for cleaner tag creation Signed-off-by: YongDo-Hyun <[email protected]>
3558776 to
ff9ba5a
Compare
Thanks for reviewing @Ryex i resolved this issue and im sorry for this. thanks. Test results: samet@192: /src/github/orgs/Project-Tick/libnbtplusplus$ cmake -S . -B build -G Ninja 100% tests passed, 0 tests failed out of 7 Total Test time (real) = 0.03 sec |
…test output handling Signed-off-by: YongDo-Hyun <[email protected]>
Signed-off-by: YongDo-Hyun <[email protected]>
…lue assignments Signed-off-by: YongDo-Hyun <[email protected]>
Signed-off-by: YongDo-Hyun <[email protected]>
|
samet@192: /src/github/orgs/Project-Tick/libnbtplusplus$ cmake -S . -B build -G Ninja 100% tests passed, 0 tests failed out of 7 Total Test time (real) = 0.01 sec |
…ing errors Signed-off-by: YongDo-Hyun <[email protected]>
Signed-off-by: YongDo-Hyun <[email protected]>
…nt masking errors Signed-off-by: YongDo-Hyun <[email protected]>
Signed-off-by: YongDo-Hyun <[email protected]>
|
~/libnbtplusplus contributeforprism ❯ cmake -S . -B build -G Ninja -DZLIB_ROOT=/nix/store/hqvsiah013yzb17b13fn18fpqk7m13cg-zlib-1.3.1-dev -DCXXTEST_INCLUDE_DIR=/nix/store/wvd95y7qi7gg7a2g506i0cfp6h87980x-cxxtest-4.4 100% tests passed, 0 tests failed out of 7 Total Test time (real) = 0.02 sec |
|
looking much better now. the only thing I'd ask (though not nessacery if we squash merge) is a clean up of the commit history to be more atomic. thanks for working with us. |
Thanks for reviewing @Ryex. I think not need for this because its only 11 commit. and all tests result is correct |
Co-authored-by: Alexandru Ionut Tripon <[email protected]> Signed-off-by: YongDo-Hyun <[email protected]>
Co-authored-by: Alexandru Ionut Tripon <[email protected]> Signed-off-by: YongDo-Hyun <[email protected]>
Co-authored-by: Alexandru Ionut Tripon <[email protected]> Signed-off-by: YongDo-Hyun <[email protected]>
Co-authored-by: Alexandru Ionut Tripon <[email protected]> Signed-off-by: YongDo-Hyun <[email protected]>
|
Hi Ryex and Trial97, I’m closing this PR as the repository is being archived due to organizational changes. Development has moved forward via a fork that is now maintained as the main repository. This PR will not be reopened. If useful, you’re welcome to manually cherry-pick or adapt any of the changes from this PR. Thanks for the reviews and the time spent on this contribution, and sorry for the inconvenience. |
This PR focuses on correctness fixes, test coverage improvements, and internal refactoring.
Changes
valueassignments to remove duplicated logic and prevent unintended truncation when assigning wider types.ozlibstream::close()by avoiding exception masking..gitignoreto ignore local editor configuration.Motivation
While working with libnbtplusplus in real world usage, I encountered several edge cases:
valueinstances could silently truncate data.These changes aim to make the library safer and more predictable without altering its public API.
Notes
NBT_BUILD_LOCAL_TEST.Signed-off-by: YongDo-Hyun [email protected]