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

Support fully qualified module names #3

Closed
wants to merge 1 commit into from
Closed

Support fully qualified module names #3

wants to merge 1 commit into from

Conversation

dustym
Copy link

@dustym dustym commented Apr 20, 2018

Given these two lines of code:

Hello::World
::Hello::World

Bust-a-gem currently only finds a symbol in the first line. This PR updates the regexes to support the latter as well.

No worries if you want to change the wordPattern to something else, in my tests the changes I made work fine, but there may be some edge cases I missed.

@dustym
Copy link
Author

dustym commented Apr 20, 2018

Hrm, tests are failing. I will look into it.

@gurgeous
Copy link
Owner

Hey, thanks for the PR! I'll take a peek this weekend. Looks simple enough. :)

@gurgeous
Copy link
Owner

OK, I think I understand what you'd like to fix and the problem with the PR.

First of all, the intent behind wordPattern is to snatch "words" from the text document and try to find matches. The current word pattern allows for symbols (which start with :). That will never provide a definition, but that's the whole point. You wouldn't want to click on :puts and have it take you to def puts by mistake. You can change the regex to allow for :: at the front and strip it off later, but we still need the ability to query (and fail) on symbols too. The regex probably needs to be changed to something like /:*[A-Za-z]... to grab all the colons at the front. Or maybe /:{0,2}[A-Za-z]....

Second, we need to fix the test. checkDefinition looks for a call site in something.rb, runs provideDefinition, and asserts that it matches the definition site. You can add another test, but you'll have to edit something.rb as well to add the call site (and maybe the definition site, but not in this case). This little test set is not documented, sorry. Add an explaining comment as well if you feel up for it. :)

Third, you should run the tests in VS code before submitting the PR. That way you'll know that things are working! Use Start Debugging, but make sure to set the current launch config to Extension Tests, not Extension which is unfortunately the default. I typically do this using the dropdown that appears in the bottom blue/orange bar when you start debugging. Then you have to stop/start debugging to get it to run Extension Tests. Maybe there's an easier way. Once you've picked the right config you can easily run the tests over and over again. This is a good skill to master because it'll set you up to do more extension PRs. You can find the VS code docs here - https://code.visualstudio.com/docs/extensions/developing-extensions, though personally I don't find them very useful on this particular topic.

Thanks @dustym - let me know if you need any more help.

@gurgeous gurgeous closed this Apr 21, 2018
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.

2 participants