monkey
monkey copied to clipboard
feat: add support to access hash index with dot separator
This PR allow to acess alternatively hash values using DOT a.b separator instead of a["b"].
I've had a quick glance it and looks reasonable, I guess I'd be curious to see if a.b.c.d.e.. works as well as a.b, but I'll test that.
Thanks for your contribution! I should be able to review more carefully, and merge, over the weekend. I did leave one minor comment already, but that's a trivial thing.
Looks like this breaks things, in two ways:
Test cases fail
$ go test ./...
? github.com/skx/monkey [no test files]
ok github.com/skx/monkey/ast (cached)
ok github.com/skx/monkey/evaluator (cached)
unterminated regular expression
--- FAIL: TestStdLib (0.00s)
lexer_test.go:441: tests[14] - tokentype wrong, expected=".", got="FIELD"
--- FAIL: TestDotMethod (0.00s)
lexer_test.go:486: tests[1] - tokentype wrong, expected=".", got="FIELD"
unterminated regular expression
FAIL
FAIL github.com/skx/monkey/lexer 0.024s
ok github.com/skx/monkey/object (cached)
# github.com/skx/monkey/parser
parser/parser.go:874:10: Sprintf call needs 1 arg but has 2 args
FAIL github.com/skx/monkey/parser [build failed]
ok github.com/skx/monkey/token (cached)
FAIL
Standard Library breaks
Once built with this merge-request applied (locally) I created a test program:
$ cat test.mon
puts( "OK\n" );
Running that:
$ ./monkey test.mon
Error calling `eval` : ERROR: index operator not support:ARRAY
Error calling `assert` : ERROR: index operator not support:ARRAY
OK
So it seems like something is broken in the standard-library, as implemented beneath data/.
These parts of the standard-library are the first to fail:
assert( "[].empty?()" );
assert( "![1,2].empty?()" );
assert( "![\"steve\",3].empty?()" );
Commenting them out results in later failures. Something odd with the filed vs. object-method. I guess there needs to be a test that the field thing applies only to hashes?
At moment, all tests has be passed. My solution to solve conflict: uses only index and call function instead of method call.
Thanks for the updates!
The changes are now bigger than I'd expected, but I've had a quick look and they seem reasonable. I'll review more thoroughly over the next day or two, but if my local code continues to work as expected I'll merge.
Thanks, again. You did a great job making the changes to the object/ implementations :)
Hello!
Although there was no error during the tests, I found it. When using builitin named functions containing DOT (example os.getenv) they do not work, because os isn't defined. For that, I needed to make some more changes. I'm doing some more tests.