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

Adds write_value to ElementWriter #579

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Conversation

popematt
Copy link
Contributor

@popematt popematt commented Jun 28, 2023

Issue #, if available:

None

Description of changes:

Adds a write_value(&self, value: &Value) function to ElementWriter so that if you have a &Value you don't need to take ownership and convert to Element in order to easily write the value.

E.g.:

fn foo<W: IonWriter>(value: &Value, writer: &mut W) {
    // Instead of this:
    writer.write_element(&value.to_owned().into());

    // Now we can write this:
    writer.write_value(value);
}

While I was at it, I also updated the documentation for write_element and write_elements, which can be used at any level, not just for top-level values.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@popematt popematt requested a review from zslayton June 28, 2023 06:43
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch coverage: 67.56% and project coverage change: -0.04 ⚠️

Comparison is base (247ac34) 83.34% compared to head (e27cad9) 83.30%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
- Coverage   83.34%   83.30%   -0.04%     
==========================================
  Files         104      104              
  Lines       19671    19705      +34     
  Branches    19671    19705      +34     
==========================================
+ Hits        16394    16416      +22     
  Misses       1739     1739              
- Partials     1538     1550      +12     
Impacted Files Coverage Δ
src/element/writer.rs 71.73% <67.56%> (-4.13%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@zslayton zslayton left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

/// Serializes a collection of [`Element`] as a series of top-level values.
/// Serializes a single [`Value`] at the current depth of the writer.
// TODO: consider extracting this to a ValueWriter trait.
fn write_value(&mut self, value: &Value) -> IonResult<()>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me wonder if we need/want a write_values(iter) sibling method, but I'm happy to punt on that.

@zslayton zslayton merged commit 779a950 into amazon-ion:main Jun 28, 2023
17 of 18 checks passed
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.

None yet

2 participants