Add Value and implement deserialize_any #53

torkleyy · 2017-09-10T15:28:02Z

Fixes #45
Fixes #48

dtolnay

I would prefer to add an enum variant in Error or ParseError to represent this. custom is meant for Deserialize impls that have no control over the Deserializer's choice of Error type.

torkleyy · 2017-09-10T17:05:12Z

@dtolnay Okay. I did it like this because bincode also uses the custom function, but I can change that. That would be a (minor) breaking change though.

dtolnay · 2017-09-10T17:13:05Z

Oh yeah good point. That may be the same reason Bincode implemented it with custom. I would temporarily do it this way and file a ticket to fix it in the next major release.

Actually we already have a ticket to make it not fail in the first place... #48.

torkleyy · 2017-09-10T17:17:15Z

Yes, that would be better. It would also fix the problem with unused values in RON files raising a "deserialize_any not supported"-error.

kvark · 2017-09-11T01:26:33Z

@torkleyy CI fails, please fix.

torkleyy · 2017-09-11T15:32:58Z

src/de/value.rs

+    {
+        let (value, variant) = data.variant()?;
+
+        Ok(Value::Enum(variant, Box::new(value)))


@dtolnay I couldn't figure out how to do deal with enums here yet.

Never mind, there is no way I could even detect an enum, so I removed Value::Enum.

kvark · 2017-09-11T15:51:14Z

hmm, do we really need to have 150+ extra lines of code to handle this case? I thought it's just a matter of failing gracefully.

torkleyy · 2017-09-11T15:58:35Z

@kvark I changed this PR to fix #48

That's why it got that big.

kvark · 2017-09-11T16:14:32Z

thanks!
bors r+

53: Add Value and implement deserialize_any r=kvark a=torkleyy Fixes #48

torkleyy · 2017-09-11T16:15:47Z

@kvark This isn't meant to be merged yet.

torkleyy · 2017-09-11T16:16:03Z

I still need to implement deserialize_any.

torkleyy · 2017-09-11T16:16:29Z

bors r-

bors · 2017-09-11T16:16:30Z

Canceled

kvark · 2017-09-11T16:52:12Z

@torkleyy I see that you used the label to tell that it's still being worked on. I think having a big large WIP in the subject is cleaner.
Edit: also having a commend in the main PR section saying "WIP because: 1. 2. 3. ..." would be great

torkleyy · 2017-09-11T17:05:12Z

Got the first test implemented and passing!

torkleyy · 2017-09-11T18:13:08Z

src/de/mod.rs

+            return visitor.visit_unit();
+        }
+
+        if self.bytes.identifier().is_ok() {


This is a little bit difficult.. I want to ignore this if it's e.g. the name of a struct, but if we're deserializing a struct, that's a problem (because the field names aren't strings like in JSON, but just unquoted identifiers). This would mean I need context information :(

@dtolnay Do you have an advice how I could solve that?

Here are 3 ways to store context information:

In self.

Passed as function arguments, for example save the identifier and pass it to the function responsible for deserializing a struct.

In a new Deserializer that wraps self and has additional fields.

…xample

kvark · 2017-09-21T00:23:52Z

src/de/mod.rs

@@ -469,10 +469,10 @@ impl<'de, 'a> de::MapAccess<'de> for CommaSeparated<'a, 'de> {
        where K: DeserializeSeed<'de>
    {
        if self.has_element()? {
-            if self.terminator == b'}' {
-                seed.deserialize(&mut *self.de).map(Some)
+            if self.terminator == b')' {


why is MapAccess separated by )?

That's because structs are also considered maps; they share some common logic. That's just a check what it is we're deserializing in order to validate the correctness of the input.

kvark · 2017-09-21T14:42:31Z

bors r+

53: Add Value and implement deserialize_any r=kvark a=torkleyy Fixes #45 Fixes #48

bors · 2017-09-21T14:45:49Z

Build succeeded

continuous-integration/travis-ci/push

Replace panic in deserialize_any with error message

3623144

dtolnay reviewed Sep 10, 2017

View reviewed changes

Add Value and ValueVisitor

a7e08af

torkleyy changed the title ~~Replace panic in deserialize_any with error message~~ Add Value and implement deserialize_any Sep 11, 2017

torkleyy added the working label Sep 11, 2017

torkleyy commented Sep 11, 2017

View reviewed changes

torkleyy added 2 commits September 11, 2017 17:54

Remove visit_enum

f09dcbe

Add Bytes::peek_or_eof

5d242bf

kvark approved these changes Sep 11, 2017

View reviewed changes

bors bot added a commit that referenced this pull request Sep 11, 2017

Merge #53

1a17f12

53: Add Value and implement deserialize_any r=kvark a=torkleyy Fixes #48

kvark changed the title ~~Add Value and implement deserialize_any~~ [WIP] Add Value and implement deserialize_any Sep 11, 2017

Implement deserialize_any

1fa6509

More tests, more fixes

5eb4272

torkleyy commented Sep 11, 2017

View reviewed changes

torkleyy added 3 commits September 14, 2017 18:38

Add IdDeserializer, fix bugs, better validation, export Value types

6202609

Fix several warnings in examples

5048eb3

Add Value serialization, restructure value module(s), add transcode e…

60c35e6

…xample

Add untagged test

749ec23

torkleyy removed the working label Sep 14, 2017

torkleyy changed the title ~~[WIP] Add Value and implement deserialize_any~~ Add Value and implement deserialize_any Sep 14, 2017

Remove generics for better compile times

4ad391b

kvark reviewed Sep 21, 2017

View reviewed changes

kvark approved these changes Sep 21, 2017

View reviewed changes

bors bot added a commit that referenced this pull request Sep 21, 2017

Merge #53

e4eab5f

53: Add Value and implement deserialize_any r=kvark a=torkleyy Fixes #45 Fixes #48

bors bot merged commit 4ad391b into ron-rs:master Sep 21, 2017

torkleyy deleted the any branch September 21, 2017 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Value and implement deserialize_any #53

Add Value and implement deserialize_any #53

torkleyy commented Sep 10, 2017 •

edited

Loading

dtolnay left a comment

torkleyy commented Sep 10, 2017

dtolnay commented Sep 10, 2017

torkleyy commented Sep 10, 2017

kvark commented Sep 11, 2017

torkleyy Sep 11, 2017

torkleyy Sep 11, 2017

kvark commented Sep 11, 2017

torkleyy commented Sep 11, 2017 •

edited

Loading

kvark commented Sep 11, 2017

torkleyy commented Sep 11, 2017 •

edited

Loading

torkleyy commented Sep 11, 2017

torkleyy commented Sep 11, 2017

bors bot commented Sep 11, 2017

kvark commented Sep 11, 2017 •

edited

Loading

torkleyy commented Sep 11, 2017

torkleyy Sep 11, 2017

torkleyy Sep 12, 2017

dtolnay Sep 12, 2017

kvark Sep 21, 2017

torkleyy Sep 21, 2017

kvark commented Sep 21, 2017

bors bot commented Sep 21, 2017

Add Value and implement deserialize_any #53

Add Value and implement deserialize_any #53

Conversation

torkleyy commented Sep 10, 2017 • edited Loading

dtolnay left a comment

Choose a reason for hiding this comment

torkleyy commented Sep 10, 2017

dtolnay commented Sep 10, 2017

torkleyy commented Sep 10, 2017

kvark commented Sep 11, 2017

torkleyy Sep 11, 2017

Choose a reason for hiding this comment

torkleyy Sep 11, 2017

Choose a reason for hiding this comment

kvark commented Sep 11, 2017

torkleyy commented Sep 11, 2017 • edited Loading

kvark commented Sep 11, 2017

torkleyy commented Sep 11, 2017 • edited Loading

torkleyy commented Sep 11, 2017

torkleyy commented Sep 11, 2017

bors bot commented Sep 11, 2017

Canceled

kvark commented Sep 11, 2017 • edited Loading

torkleyy commented Sep 11, 2017

torkleyy Sep 11, 2017

Choose a reason for hiding this comment

torkleyy Sep 12, 2017

Choose a reason for hiding this comment

dtolnay Sep 12, 2017

Choose a reason for hiding this comment

kvark Sep 21, 2017

Choose a reason for hiding this comment

torkleyy Sep 21, 2017

Choose a reason for hiding this comment

kvark commented Sep 21, 2017

bors bot commented Sep 21, 2017

Build succeeded

torkleyy commented Sep 10, 2017 •

edited

Loading

torkleyy commented Sep 11, 2017 •

edited

Loading

torkleyy commented Sep 11, 2017 •

edited

Loading

kvark commented Sep 11, 2017 •

edited

Loading