Skip to content

Add documentation for tagged enums with Serde#594

Merged
Mingun merged 1 commit into
tafia:masterfrom
Kriskras99:master
Jun 12, 2023
Merged

Add documentation for tagged enums with Serde#594
Mingun merged 1 commit into
tafia:masterfrom
Kriskras99:master

Conversation

@Kriskras99

Copy link
Copy Markdown
Contributor

As discussed in #586 here is some documentation for working with tagged enums.
I changed my implementation to a macro because it became tedious very fast (I need to deserialize an enum with 70 variants).

@Mingun Mingun left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! Can you also add a changelog entry

I also try to play with it in next days, because right now doctests are definitely will fail, because macro and usage in different blocks. We can merge both blocks or move macro implementation to it's own file and include! it in both blocks

Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
Comment thread src/de/mod.rs Outdated
@Kriskras99 Kriskras99 closed this Apr 21, 2023
@Kriskras99 Kriskras99 reopened this Apr 21, 2023
@Kriskras99

Copy link
Copy Markdown
Contributor Author

I've added a changelog entry and merged the two blocks. I've also included all your feedback except for the about using MapAccessDeserializer

@codecov-commenter

codecov-commenter commented Apr 21, 2023

Copy link
Copy Markdown

Codecov Report

Merging #594 (858118e) into master (88a2f27) will increase coverage by 0.04%.
The diff coverage is 0.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #594      +/-   ##
==========================================
+ Coverage   64.43%   64.48%   +0.04%     
==========================================
  Files          36       36              
  Lines       16919    16921       +2     
==========================================
+ Hits        10902    10911       +9     
+ Misses       6017     6010       -7     
Flag Coverage Δ
unittests 64.48% <0.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
src/de/mod.rs 69.28% <0.00%> (+0.26%) ⬆️
src/serde_helpers.rs 0.00% <ø> (ø)

... and 1 file with indirect coverage changes

Comment thread Changelog.md Outdated

### Misc Changes

- [#594]: Add documentation for using tagged enums with Serde.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Link should be actually added:

Suggested change
- [#594]: Add documentation for using tagged enums with Serde.
- [#594]: Add documentation for using tagged enums with Serde.
[#594]: https://github.com/tafia/quick-xml/pull/594

Also check cargo fmt -- it seems that there are some trailing spaces!

@Mingun

Mingun commented Apr 21, 2023

Copy link
Copy Markdown
Collaborator

I played with the implementation a bit. The reason why you've failed with MapAccessDeserializer in that that you have incorrect expectations of how the XML should be deserialized.

The types

#[derive(Deserialize, Debug, PartialEq)]
pub struct Root {
    pub one: TaggedEnum,
    pub two: TaggedEnum,
}

#[derive(Deserialize, Debug, PartialEq)]
#[serde(tag = "@tag")]
pub enum TaggedEnum {
    Spam(Spam),
    Ham,
}

#[derive(Deserialize, Debug, PartialEq)]
pub struct Spam {
    #[serde(rename = "@ham")]
    pub ham: u64,
}

describes the following XML

<root>
    <one tag="spam" ham="42" />
    <two tag="ham" />
</root>

you can make sure if you mentally replace each enumeration variant with an equivalent type: see example.

I was able to deserialize it successfully using approach with MapAccessDeserializer.

@Mingun Mingun left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I decided that since we now have a special module for helpers, add a macro there. That simple macro should help with typical tasks.

@dralley, @Kriskras99, if you have time could you please check grammar?

@Mingun Mingun merged commit 621da10 into tafia:master Jun 12, 2023
@Kriskras99

Copy link
Copy Markdown
Contributor Author

@Mingun sorry I let this pull request sit. I have reviewed the documentation and did find some grammar issues, I'll open a PR to fix them!

crapStone added a commit to Calciumdibromid/CaBr2 that referenced this pull request Jun 29, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [quick-xml](https://github.com/tafia/quick-xml) | dependencies | minor | `0.28.2` -> `0.29.0` |

---

### Release Notes

<details>
<summary>tafia/quick-xml</summary>

### [`v0.29.0`](https://github.com/tafia/quick-xml/blob/HEAD/Changelog.md#&#8203;0290----2023-06-13)

[Compare Source](tafia/quick-xml@v0.28.2...v0.29.0)

##### New Features

-   [#&#8203;601]: Add `serde_helper` module to the crate root with some useful utility
    functions and document using of enum's unit variants as a text content of element.
-   [#&#8203;606]: Implement indentation for `AsyncWrite` trait implementations.

##### Bug Fixes

-   [#&#8203;603]: Fix a regression from [#&#8203;581] that an XML comment or a processing
    instruction between a <!DOCTYPE> and the root element in the file brokes
    deserialization of structs by returning `DeError::ExpectedStart`
-   [#&#8203;608]: Return a new error `Error::EmptyDocType` on empty doctype instead
    of crashing because of a debug assertion.

##### Misc Changes

-   [#&#8203;594]: Add a helper macro to help deserialize internally tagged enums
    with Serde, which doesn't work out-of-box due to serde limitations.

[#&#8203;581]: tafia/quick-xml#581

[#&#8203;594]: tafia/quick-xml#594

[#&#8203;601]: tafia/quick-xml#601

[#&#8203;603]: tafia/quick-xml#603

[#&#8203;606]: tafia/quick-xml#606

[#&#8203;608]: tafia/quick-xml#608

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTguMCIsInVwZGF0ZWRJblZlciI6IjM1LjExOC4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1940
Reviewed-by: crapStone <crapstone01@gmail.com>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants