Add Serialization Configuration for Char and Upgrade Kotlin #75
Conversation
This file contains 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
feat: Add char serialization using Unicode code points fix: Correct serialization for chars with special characters
Also: Updated kotlinx.serialization to 1.7.0 for compatibility
|
😊 |
Him188
reviewed
Jan 30, 2025
yamlkt/src/commonMain/kotlin/net.mamoe.yamlkt/internal/YamlDecoder.kt
Outdated
Show resolved
Hide resolved
Him188
reviewed
Jan 30, 2025
yamlkt/src/commonMain/kotlin/net.mamoe.yamlkt/YamlConfigurationInternal.kt
Outdated
Show resolved
Hide resolved
Him188
reviewed
Jan 31, 2025
yamlkt/src/commonMain/kotlin/net.mamoe.yamlkt/YamlConfigurationInternal.kt
Outdated
Show resolved
Hide resolved
Him188
reviewed
Jan 31, 2025
yamlkt/src/commonMain/kotlin/net.mamoe.yamlkt/YamlConfigurationInternal.kt
Outdated
Show resolved
Hide resolved
Him188
approved these changes
Jan 31, 2025
Him188
reviewed
Jan 31, 2025
Comment on lines
71
to
80
| @Test | ||
| fun testCharEscape() { | ||
| assertEquals("\" \"", doubleChar.encodeToString<Char>(' ')) | ||
| assertEquals("\' \'", singleChar.encodeToString<Char>(' ')) | ||
| assertEquals("\' \'", PLAIN.encodeToString<Char>(' ')) | ||
| assertEquals("32", unicode.encodeToString<Char>(' ')) | ||
|
|
||
| assertEquals(' ', unicode.decodeFromString<Char>("32")) | ||
| } | ||
|
|
There was a problem hiding this comment.
IIRC class properties are handled differently than top-level values. We should also test serializable classes:
@Serializable
class Container(
val c: Char,
)
There was a problem hiding this comment.
Where should I place this test? Should it be in an existing test class or should I create a new class?
|
@Him188 Do you think we should support some special codes? For example, should we support deserialization of 'a' from |
|
If that's in the YAML spec then we are good to support them, if not, then we have to consider whether they are really needed. This PR is already good enough. Let's focus on fixing the bug first. |
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 #53 , #68
✨ Add Serialization Configuration for Char
When creating a Yaml object, you can specify the handling logic for char:
Among these,
UNICODE_CODEsupports the serialization and deserialization ofUnicode. You can now handleCharwith logic similar toByte.Additionally, escaping support for char serialization has been added to prevent certain characters (like
'\n','"', etc.) from being incorrectly input into the content. They will now be escaped with quotes.⬆️ Upgrade kotlin
Upgraded the project's kotlin version to
2.0.0. For compatibility, also updatedkotlinx.serializationto1.7.0.I encountered the same compilation issue as in #72 while attempting the update and fixed it by referring to the corresponding comment.
Therefore, theoretically, this PR should also support the content in #72 and it seems it should fix #67 as well.