Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: Serializing dictionary with ', ", . in key & #37#38

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
ITR13 wants to merge16 commits intoSamboyCoding:master
base:master
Choose a base branch
Loading
fromITR13:master

Conversation

@ITR13
Copy link
Contributor

@ITR13ITR13 commentedJan 14, 2024
edited
Loading

Fixes issue where GetTopLevelAndSubKeys splits a key when serializing.
Adds test to handle new cases.
Does not test cases with mixed " and ' due to issue explained in#37.

Fixes issue where GetTopLevelAndSubKeys splits a key when serializing.Adds test to handle new cases.Does not test cases with mixed " and ' due to issue explained inSamboyCoding#57
@coveralls
Copy link

coveralls commentedJan 14, 2024
edited
Loading

Coverage Status

coverage: 92.835% (+0.9%) from 91.909%
when pulling9061871 on ITR13:master
intoacbaa75 on SamboyCoding:master.

@ITR13ITR13 marked this pull request as draftJanuary 14, 2024 21:26
IdeallyfixesSamboyCoding#37, but current implementation has some issues.Will elaborate on issues with this "fix" in a comment.
@ITR13ITR13 changed the titlefix: Serializing dictionary with ', ", . in key.fix: Serializing dictionary with ', ", . in key & #37Jan 14, 2024
@ITR13
Copy link
ContributorAuthor

Easiest way to understand the issue is the test StringTests.EscapedQuotesInAKeyAreValid:
The key string used there is"\"a.b\"", which when passed into StringTests.EscapedQuotesInAKeyAreValid would correctly only create a single key
The problem is that the parser pre-unescapes the key to"a.b", which breaks the whole thing. It might be possible to have itnot unescape it, and instead unescape it later, but I don't know it this breaks other code.

The parser is also unable to parse strings in the form "a.b".c or ""a.b"".c, which both are allowed in the toml specification.
For that I can imagine two possible fixes, but I haven't looked at the parser yet and would not know of any other issues:

  • Make sure it doesn't look only for full quoted keys, then send the whole string back unmodified
  • Add proper splitting & escaping at parser-level, then remove GetTopLevelAndSubKeys completely and instead have keys be handled as some sort of list of strings passed into ContainsKey and GetValue.
    I haven't worked enough with text processing to know the performance or memory usage impacts of either solution though.

@ITR13ITR13 marked this pull request as ready for reviewJanuary 20, 2024 02:27
@ITR13ITR13 marked this pull request as draftJanuary 20, 2024 02:32
Added test for redefining dotted key.Made InternalPutValue always have lineNumber since it's used by the parser.Deleted now unused exception.
The current exception thrown here already shows line-number.
I think rider had a field-day when getting to generate Designer.rsAdded tests for String Reader since Backtrack wasn't covered.Added test for Document.ToString()Added test for more places it's possible to reach the end of the file.Added test for different ways of writing numbers.
@ITR13ITR13 marked this pull request as ready for reviewJanuary 20, 2024 03:42
@levicki
Copy link

This fix seems to be related to theissue I reported. Is there any ETA for it?

@ITR13ITR13 marked this pull request as draftFebruary 26, 2025 13:13
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ITR13@coveralls@levicki

[8]ページ先頭

©2009-2025 Movatter.jp