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

x.json2: add ability to decode arrays #21163

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Apr 1, 2024

Fixes #21150

Adds the ability to decode arrays, as currently it is an empty block:

The feature is integrated in a simple enough manner that it should create little interference with #21039 . It might even help with a TODO there.

@Delta456
Copy link
Member

Delta456 commented Apr 1, 2024

Isn't there a more efficient way to do this? It feels too lengthy to me.

@ttytm
Copy link
Member Author

ttytm commented Apr 1, 2024

I hope there is one but didn't found one yet. Unfortunately there is mess with accessing the comptime type of the array. Had to integrate a similar approach in toml as last option back then :/.

It is now used for arrays and maps there:

v/vlib/toml/toml.v

Lines 59 to 87 in c086bee

} $else $if field.is_array {
arr := value.array()
match typeof(typ.$(field.name)).name {
'[]string' { typ.$(field.name) = arr.as_strings() }
'[]int' { typ.$(field.name) = arr.map(it.int()) }
'[]i64' { typ.$(field.name) = arr.map(it.i64()) }
'[]u64' { typ.$(field.name) = arr.map(it.u64()) }
'[]f32' { typ.$(field.name) = arr.map(it.f32()) }
'[]f64' { typ.$(field.name) = arr.map(it.f64()) }
'[]bool' { typ.$(field.name) = arr.map(it.bool()) }
'[]toml.DateTime' { typ.$(field.name) = arr.map(it.datetime()) }
'[]toml.Date' { typ.$(field.name) = arr.map(it.date()) }
'[]toml.Time' { typ.$(field.name) = arr.map(it.time()) }
else {}
}
} $else $if field.is_map {
mut mmap := value.as_map()
match typeof(typ.$(field.name)).name {
'map[string]string' {
typ.$(field.name) = mmap.as_strings()
}
// Should be cleaned up to use the more modern lambda syntax
// |k, v| k, v.int()
// Unfortunately lambdas have issues with multiple return at the time of writing
'map[string]int' {
typ.$(field.name) = maps.to_map[string, Any, string, int](mmap, fn (k string, v Any) (string, int) {
return k, v.int()
})
}

If that's whats available atm, I hope there will be better ways soon.

@Delta456
Copy link
Member

Delta456 commented Apr 1, 2024

I hope there is one but didn't found one yet. Unfortunately there is mess with accessing the comptime type of the array. Had to integrate a similar approach in toml back then as last option :/. There it is now used for arrays and maps.

v/vlib/toml/toml.v

Lines 59 to 87 in c086bee

} $else $if field.is_array {
arr := value.array()
match typeof(typ.$(field.name)).name {
'[]string' { typ.$(field.name) = arr.as_strings() }
'[]int' { typ.$(field.name) = arr.map(it.int()) }
'[]i64' { typ.$(field.name) = arr.map(it.i64()) }
'[]u64' { typ.$(field.name) = arr.map(it.u64()) }
'[]f32' { typ.$(field.name) = arr.map(it.f32()) }
'[]f64' { typ.$(field.name) = arr.map(it.f64()) }
'[]bool' { typ.$(field.name) = arr.map(it.bool()) }
'[]toml.DateTime' { typ.$(field.name) = arr.map(it.datetime()) }
'[]toml.Date' { typ.$(field.name) = arr.map(it.date()) }
'[]toml.Time' { typ.$(field.name) = arr.map(it.time()) }
else {}
}
} $else $if field.is_map {
mut mmap := value.as_map()
match typeof(typ.$(field.name)).name {
'map[string]string' {
typ.$(field.name) = mmap.as_strings()
}
// Should be cleaned up to use the more modern lambda syntax
// |k, v| k, v.int()
// Unfortunately lambdas have issues with multiple return at the time of writing
'map[string]int' {
typ.$(field.name) = maps.to_map[string, Any, string, int](mmap, fn (k string, v Any) (string, int) {
return k, v.int()
})
}

If there is none, I hope there will be one soon.

I think we need to support checking the array in a much more functional way.

@enghitalo
Copy link
Contributor

enghitalo commented Apr 1, 2024

Very good initiative!! But I think this will cause a lot of headaches. In the end, the biggest problem today would be finding a good solution to decode Sumtypes (This will probably have to be done more "manually/handled" by the user, calling some method...). After that, array decodes became much simpler.

@enghitalo
Copy link
Contributor

I can't wait to have fun again with the new json2 decode. I hope that within the next few months I will have time to finish this project.

@ttytm
Copy link
Member Author

ttytm commented Apr 2, 2024

Very good initiative!! But I think this will cause a lot of headaches. In the end, the biggest problem today would be finding a good solution to decode Sumtypes (This will probably have to be done more "manually/handled" by the user, calling some method...). After that, array decodes became much simpler.

The change just adds array decoding for the types that the current decoder is handling. This might be better then getting empty arrays. Sumtypes might be in their own scope then.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Excellent work.

@spytheman
Copy link
Member

@enghitalo JSON does not have sumtypes. JSON does have arrays.

-> For the completion of x.json2, first class support for arrays is much more important.

@spytheman spytheman merged commit a1b6360 into vlang:master Apr 2, 2024
42 checks passed
@ttytm ttytm deleted the x.json2/decode-arrays branch April 2, 2024 02:32
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.

x.json2 can not decode properly a struct with a fixed array field
4 participants