search

fstirlitz / luaparse

A Lua parser written in JavaScript
https://fstirlitz.github.io/luaparse/
MIT License
459 stars 91 forks source link

Add onScopeIdentifierName callback #38

Closed dapetcu21 closed 7 years ago

dapetcu21 commented 8 years ago

Hi. I'm working on a Lua autocomplete package for Atom and I figured I would use luaparse.

In my use case, I need to know which identifier is local to which scope ahead of time, since the parsing might error out at the user's cursor as he's editing. That's why I added this callback, which used together with onCreateScope and onDestroyScope gives me a complete idea about the locality of identifiers without having to wait for the node to fully close (for example, a for i = 1, 10 statement would only tell me about i when the block is closed).

fstirlitz commented 7 years ago

Looks fine. onScopeIdentifierName is a bit stupid name, though. And I'd squash the three commits which add this callback. (Maybe even squash all four.)

dapetcu21 commented 7 years ago

So, do you have any suggestion of a name? Maybe onScopeIdentifier?

fstirlitz commented 7 years ago

Maybe onDeclareIdentifier (to match onCreateScope/onDestroyScope) or onLocalDeclaration.

fstirlitz commented 7 years ago

Also, it would be good to add a testcase; there's already one for the other callbacks in test/runner.js.

fstirlitz commented 7 years ago

I went ahead and squashed your commits into two, renaming the callback to onLocalDeclaration along the way (commits ab478b94caa3bc935015f6df96837dd43bc0205e and 8a24b6364360e648174fb1ac312d10d37cce2947). I've also added a testcase (c82cf9fe2c98c98df2e86f5d46d1da8382f04277). Consider it merged.

dapetcu21 commented 7 years ago

Oh. Sorry for not answering. I completely forgot about this PR and went on with my life. Thanks! :smile: