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

Infrastructure #22

Merged
merged 18 commits into from
Jun 16, 2024
Merged

Infrastructure #22

merged 18 commits into from
Jun 16, 2024

Conversation

fraya
Copy link
Member

@fraya fraya commented Jun 15, 2024

  • Basic update to infrastructure.
  • Fix an error in test suite.
  • Add documentation. Needs some love.

@fraya fraya requested a review from cgay June 15, 2024 08:06
Copy link
Member

@cgay cgay left a comment

Choose a reason for hiding this comment

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

Thanks \o/

README.md Show resolved Hide resolved

```dylan
print-json(json-table, *standard-output*);
```
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be print-json(json-table, *standard-output*, indent: 2) if you want it to pretty-print.

https://play.opendylan.org/shared/06af84b39fab129b is a complete working example. What do you think of starting to include Playground links like this in our documentation examples in general? At least until we can integrate the playground with the docs better?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to use the simplest example but using indent: is ok too, since indenting will be the more common option.
I think is a good idea to use links to play.opendylan.org in the examples. I'll do it.

Copy link
Member

Choose a reason for hiding this comment

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

Again, I'm not sure the API I created is very good. Probably defaulting to pretty-printing would have been better, and when someone realizes they want to optimize (e.g., because they're doing a JSON RPC) they could turn it off. Sigh.

for machines to parse and generate.

The json library offers two primary methods to facilitate the
conversion between JSON strings and OpenDylan tables, making it
Copy link
Member

Choose a reason for hiding this comment

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

"OpenDylan tables" should be "Dylan data structures". e.g. parse-json("2") => 2

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,38 @@
Welcome to Json's documentation!
================================
Copy link
Member

Choose a reason for hiding this comment

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

Since this will show up in the ToC sidebar it needs to be kept short, and should look like the rest of the docs, so just

json
====

(I'm fine with either upper or lower case since JSON is an acronym, but most package titles are lower case currently.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.


:gf:`parse-json`
This method takes a JSON-formatted string and converts it into an
OpenDylan table. This function is useful when you need to process
Copy link
Member

Choose a reason for hiding this comment

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

"OpenDylan" should be "Open Dylan" everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@@ -0,0 +1,189 @@
Json library reference
Copy link
Member

Choose a reason for hiding this comment

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

"json Library Reference"

Copy link
Member Author

Choose a reason for hiding this comment

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

is ok "Json Library Reference"?

Copy link
Member

Choose a reason for hiding this comment

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

I think "Json" is a very odd capitalization for an acronym, but it's up to you.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

.. code-block:: dylan

let data = """{"a": 1, "b": 2,}""";
let parsed = parse-json(data);
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs strict?: #f to work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

whitespace. If an integer, then use pretty printing and output
*indent* spaces for each indent level.

If `sort-keys:` is true, output object keys in lexicographical
Copy link
Member

Choose a reason for hiding this comment

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

sort-keys?:

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.


Override this to print your own objects in JSON format. It can be
implemented by converting objects to built-in Dylan types (tables,
collections, etc) and calling *print* on those objects, or by
Copy link
Member

Choose a reason for hiding this comment

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

"print" should be "print-json" here, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Fix error in example and adds links to play.opendylan.org
Short title since this will show up in the ToC sidebar and it needs to
be kept short, and should look like the rest of the docs.
@fraya fraya requested a review from cgay June 16, 2024 10:50
@fraya fraya merged commit d8ee258 into dylan-lang:master Jun 16, 2024
2 checks passed
@fraya fraya deleted the infrastructure branch June 16, 2024 18:46
@fraya fraya mentioned this pull request Jun 20, 2024
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.

2 participants