monkey icon indicating copy to clipboard operation
monkey copied to clipboard

feat: add support to access hash index with dot separator

Open moisespsena opened this issue 4 years ago • 6 comments

This PR allow to acess alternatively hash values using DOT a.b separator instead of a["b"].

moisespsena avatar Jan 22 '21 12:01 moisespsena

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.

skx avatar Jan 22 '21 12:01 skx

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/.

skx avatar Jan 22 '21 15:01 skx

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?

skx avatar Jan 22 '21 19:01 skx

At moment, all tests has be passed. My solution to solve conflict: uses only index and call function instead of method call.

moisespsena avatar Jan 23 '21 01:01 moisespsena

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 :)

skx avatar Jan 24 '21 09:01 skx

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.

moisespsena avatar Jan 24 '21 11:01 moisespsena