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

feat: add support to access hash index with dot separator #75

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

moisespsena
Copy link

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

parser/parser.go Outdated Show resolved Hide resolved
@skx
Copy link
Owner

skx commented Jan 22, 2021

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 skx self-assigned this Jan 22, 2021
@skx
Copy link
Owner

skx commented Jan 22, 2021

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
Copy link
Owner

skx commented Jan 22, 2021

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?

@moisespsena
Copy link
Author

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

@skx
Copy link
Owner

skx commented Jan 24, 2021

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

@moisespsena
Copy link
Author

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.

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.

None yet

2 participants