Skip to content
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

Do not allow consequent primitives in $value fields and top-level #823

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

Mingun
Copy link
Collaborator

@Mingun Mingun commented Oct 15, 2024

This PR changes serialization of $value fields with types like Vec<String> or (u32, u32, u32). Previously this fields would be serialized without delimiters between elements which is unlikely what you want. Now serialization of that types would return SeError::Unsupported.

Example:

struct Xml {
  #[serde(rename = "$value")]
  seq: Vec<String>,

  #[serde(rename = "$value")]
  tuple: (bool, usize, String),
}

Currently it is based on #820, only the last commit implements the change.

This also partially handle this #497 (comment).

@Mingun Mingun added serde Issues related to mapping from Rust types to XML arrays Issues related to mapping XML content onto arrays using serde labels Oct 15, 2024
@Mingun Mingun requested a review from dralley October 15, 2024 19:28
Copy link
Collaborator

@dralley dralley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be helpful to mention the "consequent primitives" caveat in the documentation.

@Mingun Mingun force-pushed the do-not-allow-consequent-primitives branch from 8b6e8c6 to a9391f3 Compare October 18, 2024 20:25
@Mingun
Copy link
Collaborator Author

Mingun commented Oct 18, 2024

I think, it is already mentionen in $value/Primitives and sequences of primitives part of documentation (but I forgot to update this part, now fixed). If you have better place where we can put it or describe it better, make a PR. Perhaps my gaze is too blurry.

@Mingun Mingun merged commit b96b876 into tafia:master Oct 18, 2024
7 checks passed
@Mingun Mingun deleted the do-not-allow-consequent-primitives branch October 18, 2024 20:29
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 82.92683% with 7 lines in your changes missing coverage. Please review.

Project coverage is 60.21%. Comparing base (39b5905) to head (a9391f3).
Report is 22 commits behind head on master.

Files with missing lines Patch % Lines
src/de/mod.rs 0.00% 7 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #823      +/-   ##
==========================================
+ Coverage   60.08%   60.21%   +0.12%     
==========================================
  Files          41       41              
  Lines       15975    16021      +46     
==========================================
+ Hits         9599     9647      +48     
+ Misses       6376     6374       -2     
Flag Coverage Δ
unittests 60.21% <82.92%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays Issues related to mapping XML content onto arrays using serde serde Issues related to mapping from Rust types to XML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants