Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Having two while-loops in a file breaks symbol location #444

Closed
peret opened this issue Mar 3, 2019 · 8 comments
Closed

Having two while-loops in a file breaks symbol location #444

peret opened this issue Mar 3, 2019 · 8 comments
Assignees

Comments

@peret
Copy link
Contributor

peret commented Mar 3, 2019

Your environment

  • vscode-ruby version: 0.20.0
  • Ruby version: 2.4.1
  • Ruby version manager (if any): rvm
  • VS Code version: 1.31.0
  • Operating System: Ubuntu
  • Using language server? No

Expected behavior

vscode-ruby can locate all method names in a file.

Actual behavior

Under some conditions, when using rubyLocate for locating symbols, not all symbols are listed. E.g. in this file:

class Example
  def first_method
    while true
      a = 1
      break
    end

    while false
      "do nothing"
    end
  end

  def second_method
  end
end

the extension will not find second_method for symbol lookup. If you remove one of the while-loops, it'll work. I tracked this down to a bug in the parsing of while(-modifiers) in ruby-method-locate and also provided a fix: HookyQR/ruby-method-locate#1
Unfortunately, it looks like that repository might not be maintained anymore? It looks like @HookyQR used to be a contributor to this repo as well, but wasn't active in a while?

I'm creating this issue here because it's really annoying behavior in vscode-ruby and I want to move this along.
For example, would you be willing to use my fork of ruby-method-locate, if we can't get the PR in the original repo merged?

Thanks!

@wingrunr21
Copy link
Collaborator

I am not against switching to your fork in the interim. I'd like it to be published to NPM though.

FYI I am not planning on supporting rubyLocate as a symbol provider when I implement that in the language server. I have considerably better symbol information available in the AST than what rubyLocate provides.

@peret peret closed this as completed Mar 5, 2019
@wingrunr21
Copy link
Collaborator

@peret sorry if that was unclear. If you can publish your fork to NPM we can switch the project to use it (you can submit a PR for that too!)

@peret
Copy link
Contributor Author

peret commented Mar 5, 2019

Whoops, definitely didn't mean to close this issue 😅 Looks like I just randomly hit the button.
Sorry, nothing unclear about your answer. I think I'm going to wait a few more days to see if the PR in rubyLocate gets merged (got a reply from the maintainer now), before publishing a fork.
Good to know that it will be replaced with the language server though!

@peret peret reopened this Mar 5, 2019
@HookyQR
Copy link
Contributor

HookyQR commented Mar 5, 2019

ruby-method-locate 0.0.6 has been published. Thanks for your PR @peret. Sorry it took SOOOO long.

@wingrunr21
Copy link
Collaborator

Sweet, I'll get it updated here

@wingrunr21 wingrunr21 self-assigned this Mar 5, 2019
@HookyQR
Copy link
Contributor

HookyQR commented Mar 5, 2019

The extension will need a version bump (post npm install) to shove the new version into the package.

@wingrunr21
Copy link
Collaborator

Yes I know

@HookyQR
Copy link
Contributor

HookyQR commented Mar 5, 2019

You're just too quick for me. 😉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants