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

Is require_paths relative to workspace? #162

Closed
thomthom opened this issue Feb 25, 2019 · 5 comments
Closed

Is require_paths relative to workspace? #162

thomthom opened this issue Feb 25, 2019 · 5 comments

Comments

@thomthom
Copy link

I'm trying to help out on a reported issue in a VSCode + Solargraph setup example I've provided:
SketchUp/sketchup-ruby-api-tutorials#15

Given a .solargraph.yml like this: (Example project at SketchUp/sketchup-ruby-api-tutorials#15 (comment))

require_paths:
- "C:/Program Files/SketchUp/SketchUp 2018/Tools"
- src

# This will load the stubs for the SketchUp API.
require:
- sketchup-api-stubs/autoload/sketchup.rb

And a src/sample.rb like this: (Along with a dummy src/sample/main.rb)

require 'sketchup.rb'
require 'sample/main'

module Example

  def self.hello
    model = Sketchup.active_model
    model.save
  end

end

require 'sketchup.rb is flagged as not resolved:
image

sketchup.rb is located in the C:/Program Files/SketchUp/SketchUp 2018/Tools directory.

I've used this setup for my own projects, but not seen this issue. For example, this project have similar setup but does not yield the require warning:

https://github.com/thomthom/true-bend

I looked around in the source code for solargraph, and I now I'm wondering how I don't see that in all my projects. There are two things which appear to not make this scenario work:

  1. It appear that .rb is automatically appended to the path when checking if a path can be required:

# True if the path resolves to a file in the workspace's require paths.
#
# @param path [String]
# @return [Boolean]
def would_require? path
require_paths.each do |rp|
return true if File.exist?(File.join(rp, "#{path}.rb"))
end
false
end

In my case, require 'sketchup.rb includes the file extension for legacy reasons. So it then looks like solargraph looks for sketchup.rb.rb. Am I reading the code right?

  1. It appear the require_paths in .solargraph.yml is always prefixed with the path of the workspace?

# Get additional require paths defined in the configuration.
#
# @return [Array<String>]
def configured_require_paths
return ['lib'] if directory.empty?
return [File.join(directory, 'lib')] if config.require_paths.empty?
config.require_paths.map{|p| File.join(directory, p)}
end

So if I understand the code correctly, solargraph expects require statements to omit .rb and require_paths to be relative to the workspace? If that is correct, then I'm confused to why I don't see the same require warning in my other projects. Any ideas?

@thomthom
Copy link
Author

Oooh.....

I just realized that I have inadvertently omitting require_not_found reporters:

https://github.com/thomthom/true-bend/blob/07caec6f95759fad7e8f6902527fdccf10011495/.solargraph.yml#L8-L9

require_paths:
- "C:/Program Files/SketchUp/SketchUp 2018/Tools"
- src

require:
- sketchup-api-stubs/autoload/sketchup.rb

reporters:
- rubocop

Aaand... I just realized I've had this discovery before..... castwide/vscode-solargraph#56 (comment)

I'm sorry for the noise.

@thomthom
Copy link
Author

Btw, would it be to naive to allow for absolute paths in require_paths by simply adjusting Workspace#configure_require_paths? Or are there other things that would have to adjust as well?

Would you accept a PR that let Solargraph append .rb only if there isn't already a .rb extension in the file path?

@castwide
Copy link
Owner

That sounds like a good change, yes. There might need to be an accompanying change in the mapping process that looks for unresolved require paths, but I wouldn't expect it to be too troublesome.

@shepherdjerred
Copy link

Has support for absolute paths in require_paths been added? I can't find any documentation for this configuration option.

@castwide
Copy link
Owner

Not yet, but since you reminded me, I added it to the roadmap.

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

No branches or pull requests

3 participants