close
Skip to content

feat(stdlib): Json value access utils#2150

Merged
ospencer merged 4 commits into
mainfrom
alex/json-value-utils
Nov 7, 2024
Merged

feat(stdlib): Json value access utils#2150
ospencer merged 4 commits into
mainfrom
alex/json-value-utils

Conversation

@alex-snezhko
Copy link
Copy Markdown
Member

Closes #1877

Copy link
Copy Markdown
Member

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

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

The implementation itself mostly looks great to me with a few minor notes.

I personally feel like as we are likely to add other libraries like Toml, Yaml and maybe more the searching would be better provided as a seperate library and we could have a toSearchable or something in the json library that converts the format. The searching semanitcs between bassically every config language is the same and most of the datatypes are the same.

Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
@ospencer
Copy link
Copy Markdown
Member

I'd love for us to have combinators to search JSON, somewhat similar to what this TOML library does: https://github.com/ocaml-toml/To.ml?tab=readme-ov-file#lenses

Ours wouldn't need to be quite that complex, and I think you'd only need some minor tweaks.

@alex-snezhko
Copy link
Copy Markdown
Member Author

@ospencer switched over to using lenses

Comment thread stdlib/json.gr
* @since v0.7.0
*/
provide let prop = propertyName =>
{ get: json => match (json) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unrelated to this PR but I don't really like how the formatter handles these records here

Copy link
Copy Markdown
Member

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

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

This looks sick.

Comment thread stdlib/json.gr
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr
Copy link
Copy Markdown
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

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

This is beautiful! I'm excited to use this. Just a few minor doc changes and this is good to go.

Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
Comment thread stdlib/json.gr Outdated
@alex-snezhko alex-snezhko force-pushed the alex/json-value-utils branch from e99cde8 to ab6cd43 Compare October 21, 2024 00:57
@alex-snezhko
Copy link
Copy Markdown
Member Author

@ospencer done

@ospencer ospencer added this pull request to the merge queue Nov 7, 2024
Merged via the queue into main with commit 72cc978 Nov 7, 2024
@ospencer ospencer deleted the alex/json-value-utils branch November 7, 2024 05:30
@github-actions github-actions Bot mentioned this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stdlib: add a searchable json api

3 participants