castwide / vscode-solargraph

A Visual Studio Code extension for Solargraph.
Other
425 stars 23 forks source link

Exclude config has no effect #93

Closed dogweather closed 5 years ago

dogweather commented 5 years ago

In VS Code, when I open an rspec file, it's covered by an exclude: in my .solargraph.yml. However, Solargraph lints it anyhow.

My config:

---
include:
- "**/*.rb"
exclude:
- spec/**/*
- vendor/**/*
- ".bundle/**/*"
require: []
domains: []
reporters:
- rubocop
- require_not_found
plugins: []
require_paths: []
max_files: 5000

Ruby 2.6.0 Solargraph 0.30.1 VS Code 1.30.1

castwide commented 5 years ago

The include and exclude configurations determine whether a file should be mapped as part of the workspace. Test folders are excluded by default because it's typically not desirable to include test classes in the rest of your project.

Diagnostics work a little differently. The language server assumes you want to run diagnostics on any Ruby file you open in the editor, even if it's not part of the workspace. This behavior matches my particular preference--I still want to be informed of style offenses in a spec file, even though it's not mapped--but I can see how that might not always be desirable. #94 is a good example of an erroneous result. A similar issue specific to RuboCop was discussed in castwide/solargraph#93.

I'm not sure of the best way to resolve this one. The first idea that comes to mind is adding another configuration option, but making it work for all reporters instead of RuboCop only.

Another possibility is allowing users to add .solargraph.yml files to subdirectories. This has been discussed before as well. The multi-root workspace feature introduced in gem v0.30.0 makes it feasible.

I'm open to suggestions here.

dogweather commented 5 years ago

Thanks! By the way, I was actually only trying to do this to workaround this problem: #94 .

castwide commented 5 years ago

@dogweather Thanks, I appreciate the clarification. The error in #94 is a legitimate bug under some circumstances, but is also sometimes safe to ignore. If you're looking at a spec file in a project, it should probably be ignored, but it should also not cause an LSP error. We need to choose when to capture or ignore those issues based on the project's configuration. I'm trying to make it work for the most common situations, but I'm also missing use cases that I didn't anticipate. Any input you have is welcome.

dogweather commented 5 years ago

Thanks!