Add TOONDecoder#11
Conversation
mattt
left a comment
There was a problem hiding this comment.
Thanks for re-opening this, @alexey1312. I left a few comments about the implementation, but this is looking really good.
ce7f3ba to
d5e3d27
Compare
# Conflicts: # .github/workflows/ci.yml
- Remove configurable indent property; auto-detect from first indented line - Add .automatic path expansion mode as new default (falls back on collision) - Consolidate specVersion into top-level toonSpecVersion constant - Remove separate TOONEncoder/TOONDecoder library exports from Package.swift - Reduce default maxDepth from 128 to 32 for stack safety - Simplify README with unified quick start example
d5e3d27 to
9a89d7e
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive TOON format decoding capabilities to the Swift package, completing the encode/decode cycle. The decoder is ported from an external repository and adapted to work alongside the existing TOONEncoder, with full TOON specification 3.0 compliance.
Key Changes:
- Added
TOONDecoderclass with support for all TOON format features including tabular arrays, path expansion, and configurable security limits - Extracted shared
Valueenum andIndexedCodingKeyto a separateValue.swiftfile for reuse between encoder and decoder - Added
toonSpecVersionconstant inVersion.swiftto replace the encoder-specificTOONEncoder.specVersion
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
Sources/ToonFormat/Version.swift |
New file defining the shared toonSpecVersion constant ("3.0") |
Sources/ToonFormat/Value.swift |
New file with extracted Value enum (intermediate representation) and IndexedCodingKey struct, now shared between encoder and decoder |
Sources/ToonFormat/Decoder.swift |
New comprehensive decoder implementation with parser, error types, decoding containers, and security limits |
Sources/ToonFormat/Encoder.swift |
Refactored to use shared Value enum from Value.swift; improved documentation; made String extension internal (previously private) |
Tests/ToonFormatTests/EncoderTests.swift |
Updated test suite name from "TOONEncoder Tests" to "Encoder Tests" and changed version check to use toonSpecVersion |
Tests/ToonFormatTests/DecoderTests.swift |
New comprehensive test suite with 167 tests covering primitives, objects, arrays, path expansion, round-trip encoding/decoding, error cases, and security limits |
README.md |
Updated with decoder features, round-trip usage examples, decoding limits configuration, and updated package version to 0.3.0 |
Package.swift |
Removed boilerplate comments for cleaner appearance |
Comments suppressed due to low confidence (1)
Sources/ToonFormat/Encoder.swift:1453
- Inconsistent access control modifiers in String extension. The properties
isNumericLikeandisPaddedWithWhitespaceare markedfileprivate(lines 1378, 1387), whileescaped,isSafeUnquoted,isValidUnquotedKey, andisValidIdentifierSegment(lines 1369, 1391, 1441, 1447) have no explicit modifier and default tointernal. Since these methods are only used within the Encoder file, they should all be consistently marked asfileprivateto prevent accidental usage from other files in the module.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes a spurious test failure on Swift 6.0.3 on Linux only
mattt
left a comment
There was a problem hiding this comment.
Fantastic work on this, @alexey1312! I'm extremely happy with how all of this turned out. Thank you so much for your help. I think our Swift implementation stacks up quite favorably to the others.
|
Requesting a review from @johannschopplich since I was the last pusher and my review isn't sufficient for merging this PR. |
johannschopplich
left a comment
There was a problem hiding this comment.
LGTM!
@mattt Actually, I'm not sure my review is sufficient either. If you mean the PR review requirements – I have set it to 0. So you can merge your own PRs and others, since you are the sole maintainer of the community-driven implementation. I'm only here for governance on some core files or README changes. :)
Description
This PR adds
TOONDecoderclass for parsing TOON format data into SwiftDecodabletypes, completing the full encode/decode cycle for TOON format in Swift. The decoder is ported from alexey1312/TOONDecoder and adapted to the ToonFormat package structure from #5.Type of Change
Related Issues
Closes #6
Changes Made
TOONDecoderclass with full TOON spec 3.0 complianceTOONDecodingErrorenum with detailed error typesDecodingLimitsfor security (maxDepth: 32, maxObjectKeys: 10,000, maxArrayLength: 100,000)PathExpansionwith.automaticmode as default (falls back gracefully on collision)toonSpecVersionconstant for spec version declarationREADME.mdwith round-trip example in Quick Start sectionSPEC Compliance
Testing
Test Output
Code Quality
swift test- all tests passswift-format- code follows format guidelinesChecklist
Performance Impact
Breaking Changes
Screenshots / Examples
Tabular array decoding:
Additional Context