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

Allow `json` to be called on all JSON types #54

Closed
odino opened this Issue Dec 21, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@odino
Copy link
Collaborator

odino commented Dec 21, 2018

currently, "[1, 2, 3]".json() does not work

@odino odino added the bug label Dec 21, 2018

@odino odino added this to the future milestone Dec 21, 2018

@kseistrup

This comment has been minimized.

Copy link

kseistrup commented Dec 27, 2018

Related: abs doesn't seem to recognize null:

⧐  "{\"x\": null}".json()
ERROR: identifier not found: null

I will be happy to open a separate issue if it doesn't belong here.

@odino

This comment has been minimized.

Copy link
Collaborator Author

odino commented Dec 27, 2018

@kseistrup

This comment has been minimized.

Copy link

kseistrup commented Dec 27, 2018

For null, please see #85.

@odino odino modified the milestones: cauldron, 1.0 Jan 22, 2019

odino added a commit that referenced this issue Jan 22, 2019

Allow `.json(...)` to be called on all types, closes #54
```
⧐  type('{}'.json())
HASH
⧐  type('[]'.json())
ARRAY
⧐  type('"hello"'.json())
STRING
⧐  type('1'.json())
NUMBER
```

In this PR I had to revert some of the changes we made in the tests
earlier on. Using `strings.Contains` in the tests is very dangerous,
and I reverted back to equality for most tests -- with the exception
of the tests that check for error messages that use `strings.HasPrefix`
which should be a bit more robust. The problem with `strings.Contains`
is that is a tests expects a string (`"hello"`) and instead an error
is thrown (`"Unable to call function: hello"`) the test will silently
pass since it finds our expected string inside the error message. With
`strings.HasPrefix` we limit these cases considerably.

The end goal would be to make sure
we can actually differentiate when ABS returns a string and an error string,
but for now this is something we can work with.

odino added a commit that referenced this issue Jan 22, 2019

Allow `.json(...)` to be called on all types, closes #54
```
⧐  type('{}'.json())
HASH
⧐  type('[]'.json())
ARRAY
⧐  type('"hello"'.json())
STRING
⧐  type('1'.json())
NUMBER
```

In this PR I had to revert some of the changes we made in the tests
earlier on. Using `strings.Contains` in the tests is very dangerous,
and I reverted back to equality for most tests -- with the exception
of the tests that check for error messages that use `strings.HasPrefix`
which should be a bit more robust. The problem with `strings.Contains`
is that is a tests expects a string (`"hello"`) and instead an error
is thrown (`"Unable to call function: hello"`) the test will silently
pass since it finds our expected string inside the error message. With
`strings.HasPrefix` we limit these cases considerably.

The end goal would be to make sure
we can actually differentiate when ABS returns a string and an error string,
but for now this is something we can work with.

odino added a commit that referenced this issue Jan 23, 2019

Allow `.json(...)` to be called on all types, closes #54
```
⧐  type('{}'.json())
HASH
⧐  type('[]'.json())
ARRAY
⧐  type('"hello"'.json())
STRING
⧐  type('1'.json())
NUMBER
```

In this PR I had to revert some of the changes we made in the tests
earlier on. Using `strings.Contains` in the tests is very dangerous,
and I reverted back to equality for most tests -- with the exception
of the tests that check for error messages that use `strings.HasPrefix`
which should be a bit more robust. The problem with `strings.Contains`
is that is a tests expects a string (`"hello"`) and instead an error
is thrown (`"Unable to call function: hello"`) the test will silently
pass since it finds our expected string inside the error message. With
`strings.HasPrefix` we limit these cases considerably.

The end goal would be to make sure
we can actually differentiate when ABS returns a string and an error string,
but for now this is something we can work with.

@odino odino closed this Jan 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment