Add Writer::write_serializable #609
Conversation
dralley
force-pushed
the
write-serializable
branch
from
a8f8bf6
to
8736d7b
Compare
dralley
force-pushed
the
write-serializable
branch
from
8736d7b
to
1f80b16
Compare
|
@Mingun Ignore the commit history, I will clean it up (and fix the changelogs, etc.) prior to marking this officially ready for review. I have left a few open questions on the API. I moved the function from ElementWriter to Writer for all the reasons laid out on the other PR, and because I think it would only add corner cases. For instance, I don't think there is a need to add your own custom attributes on top of what the object would already output during serialization. Do you agree? I left a few other comments also. |
dralley
force-pushed
the
write-serializable
branch
from
1f80b16
to
cc1958f
Compare
src/se/mod.rs
Outdated
| &mut self, | ||
| indent_char: char, | ||
| indent_size: usize, | ||
| initial_indent_amt: usize, |
There was a problem hiding this comment.
I find this part a bit unsatisfying, I'd rather it be an initial "indent level" that is multiplied by the indent_size to get the initial indent.
There was a problem hiding this comment.
Agreed that this would be better.
I think is would be even better to create a new internal method to just set Indent on serializer directly. Then no need to introduce new parameter here.
Codecov Report
❗ 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 #609 +/- ##
==========================================
+ Coverage 64.48% 64.69% +0.21%
==========================================
Files 36 36
Lines 16921 17014 +93
==========================================
+ Hits 10911 11007 +96
+ Misses 6010 6007 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
dralley
force-pushed
the
write-serializable
branch
from
cc1958f
to
f23517e
Compare
There was a problem hiding this comment.
I moved the function from ElementWriter to Writer for all the reasons laid out on the other PR, and because I think it would only add corner cases. For instance, I don't think there is a need to add your own custom attributes on top of what the object would already output during serialization. Do you agree?
Yes, this is probably a very niche use case, so the proposed API looks good.
src/se/mod.rs
Outdated
| &mut self, | ||
| indent_char: char, | ||
| indent_size: usize, | ||
| initial_indent_amt: usize, |
There was a problem hiding this comment.
Agreed that this would be better.
I think is would be even better to create a new internal method to just set Indent on serializer directly. Then no need to introduce new parameter here.
dralley
changed the title
write_serializable_content on element writer
dralley
force-pushed
the
write-serializable
branch
3 times, most recently
from
35be8a4
to
3923c63
Compare
dralley
force-pushed
the
write-serializable
branch
4 times, most recently
from
0d878c2
to
dbd1076
Compare
|
@Mingun Ready |
There was a problem hiding this comment.
Good work. I left some comments on things that I think would be good to have, but nothing blocking.
If you decide to add additional commits, it would be also good to put renaming current_indent_len with documenting fields into its own commit. I like when different things not mixed in one commit
dralley
force-pushed
the
write-serializable
branch
2 times, most recently
from
909cd12
to
fcdcdf2
Compare
Allow serializing individual objects using serde with the raw Writer API closes tafia#610
dralley
force-pushed
the
write-serializable
branch
from
fcdcdf2
to
e067431
Compare
This introduces a new write_serializable_content method in order to serialize arbitrary types through serde within manual manipulation of an XML writer.