search

NetLogo / Tortoise

Compiler and runtime engine for NetLogo models that runs in JavaScript 🐢
https://netlogoweb.org
Other
56 stars 27 forks source link

Addressed issues raised by Jeremy B. #231

Closed CIVITAS-John closed 2 years ago

CIVITAS-John commented 2 years ago

Hi there,

I hope this commit could address your issues, as I manually tested those new primitives.

Thanks!

CIVITAS-John commented 2 years ago

At this moment, I think [10, 20, 30] will return a list - which is probably non-canonical, but some users might indeed need it.

For other requested changes, I agree.

LaCuneta commented 2 years ago

Right now table:from-json "[10, 20, 30]" would error, as it checks that the result is a table first. You might be able to talk me into returning a list being okay there, but my gut instinct is pointing people to the CSV extension would be a better way to handle that case.

CIVITAS-John commented 2 years ago

Hi Jeremy,

Could you check out this version of pull request and see if it matches your expectation?

John

LaCuneta commented 2 years ago

@CIVITAS-John The code changes all look good, but I did want to see a couple negative error conditions added to the language tests for invalid input. If you want to omit the array/list example for now, that's fine, but at least one example of the failure going to-json and from-json.

CIVITAS-John commented 2 years ago

Hi Jeremy,

I might need your help to correctly format the two remaining issues.

It seems that the JSON parser one is a bit hard, since the error string could be different based on different environments? Or should we catch underlying errors and throw them?