Skip to content

Conversation

@richardmcmillen
Copy link
Contributor

Tree sitter sees these as singleton_methods and this was missing from
the namedFunction and functionName definitions.

Checklist


Fixes #760

@richardmcmillen richardmcmillen requested a review from pokey as a code owner June 23, 2022 03:11
@richardmcmillen richardmcmillen force-pushed the 760-fix-class-method-scope-ruby branch 4 times, most recently from eb31331 to 552e01a Compare June 23, 2022 03:22
Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! See inline comment, and also

  • Worth adding a test for "arg" within an arg list for one of these functions

I'll also give @Will-Sommers a chance to have a look

namedFunction: "method",
functionName: "method[name]",
namedFunction: ["method", "singleton_method"],
functionName: ["method[name]", "singleton_method[name]"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add to name (line 187 below), and add a test for that. Just one test for that one is fine

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this suggestion in the issue @richardmcmillen!

@richardmcmillen richardmcmillen force-pushed the 760-fix-class-method-scope-ruby branch from 552e01a to ffa39d9 Compare June 23, 2022 08:31
Tree sitter sees these as `singleton_methods` and this was missing from
the `namedFunction` and `functionName` definitions.
@richardmcmillen richardmcmillen force-pushed the 760-fix-class-method-scope-ruby branch from ffa39d9 to fd47787 Compare June 23, 2022 08:34
@richardmcmillen
Copy link
Contributor Author

Worth adding a test for "arg" within an arg list for one of these functions

I added a new test case which I think covers this if I understand correctly, and make the other change as well.

Copy link
Collaborator

@Will-Sommers Will-Sommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thank you!

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep looks good to me too thanks!

@pokey pokey merged commit a22b4bf into cursorless-dev:main Jun 23, 2022
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.

Scope funk doesn't match on Class methods - ruby

3 participants