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

Fix empty enum again #16

Merged
merged 2 commits into from
Mar 1, 2017
Merged

Fix empty enum again #16

merged 2 commits into from
Mar 1, 2017

Conversation

kevinresol
Copy link
Member

Closes #14

@@ -91,6 +91,9 @@ class Macro {
}

static public function getRepresentation(t:Type, pos:Position) {

if(t.match(TMono(_))) return None;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to guard infinite loop if something happens to be Unknown<0>.
For example, it happens when doing tink.Json.stringify(None), where the type is Option<Unknown>

@Gama11
Copy link
Contributor

Gama11 commented Feb 26, 2017

The '{ "EnumValue": {} }' case now works. Shouldn't '{ "EnumValue" }' work too though? For that I get:

Error: Error#422: Expected : at character 14 in { "EnumValue" ----> } <---- @ tink.json.BasicParser.die:237

@kevinresol
Copy link
Member Author

kevinresol commented Feb 26, 2017

'{ "EnumValue" }' is invalid json. This lib tries to represent haxe data with json syntax + macro magic. It doesn't modify the json synxtax itself. Without haxe macro, you can still parse it. Only it may not be very meaningful.

@Gama11
Copy link
Contributor

Gama11 commented Feb 26, 2017

Right you are, I was thinking about something like this:

class Main {
    public static function main() {
        var o:Struct = tink.Json.parse('{ "e": "EnumValue" }');
        trace(o);
    }
}

typedef Struct = {
    var e:TestEnum;
}

enum TestEnum {
    EnumValue;
}

@kevinresol
Copy link
Member Author

kevinresol commented Feb 26, 2017

I think for now the best thing you can do is this

enum TestEnum {
    @:json({type:'EnumValue'}) EnumValue;
}
trace(tink.Json.stringify({e:EnumValue})); // {"e":{"type":"EnumValue"}}

@kevinresol
Copy link
Member Author

kevinresol commented Feb 26, 2017

'{ "e": "EnumValue" }'

It is because the reader reads all enum values as object, so it always expects a {. (right after the colon in this case) It is a runtime task to determine a enum value to have arguments or not. So I don't think it is possible to support that.

@Gama11
Copy link
Contributor

Gama11 commented Feb 26, 2017

It is a runtime task to determine a enum value to have arguments or not

How so? You should know whether an enum constructor has arguments or not at compile-time, you know the enum's type after all.

Note that I have very little idea how tink_json works internally. :)

@kevinresol
Copy link
Member Author

kevinresol commented Feb 26, 2017

It is a runtime task to determine a enum value to have arguments or not

When the type is a Enum, the macro basically generate a piece of code that say: "hey we are expecting a enum, let's read it as an object and get the constructor name from within it". So, the parser will always expect a { at that point.
In other words, you can't tell the code to expect " instead of { just for a no-arg enum value. Because you can't know which enum constructor it is before you parse it.

@Gama11
Copy link
Contributor

Gama11 commented Feb 26, 2017

So it only checks the next character, meaning "expect either { or "EnumValue"" wouldn't work?

@kevinresol
Copy link
Member Author

So it only checks the next character, meaning "expect either { or "EnumValue"" wouldn't work?

Well, that should work in theory. What I mentioned is the current status quo of the parser.

@back2dos back2dos merged commit 92c040b into master Mar 1, 2017
@kevinresol kevinresol deleted the noarg_enum branch May 17, 2017 14:12
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

3 participants