pragmagic / vscode-nim

An extension for VS Code which provides support for the Nim language.
Other
237 stars 37 forks source link

add support for globs in nim.project setting #90

Open Aldlevine opened 6 years ago

Aldlevine commented 6 years ago

Instead of listing out all project files in vscode settings, this lets you specify glob patterns to match multiple files.

{
  "nim.project": ["src/**/*.nim"]
}
kosz78 commented 6 years ago

Using this approach it is very easy make mistake to include dont required project files, User should be very carefull with including project file because each project file creates separate nimssuggest process that consume a lot of memory, also for when the plugin perform linting it execute nim check for each project file. I guess include each project file separately give more control than patterns

Aldlevine commented 6 years ago

I see that now. I just tested this out on a project with ~16 files and it is very resource heavy. I agree that it's probably not best to have such a feature, as it would only be usable for small projects where it would have the least benefit.

That said, I did notice that if you specify a single source file then all imported modules appear to get checked as well, and it is very performant in comparison. What if the extension generates a temp source file which imports the matched modules and runs everything against that file? It's certainly not an elegant solution, but it seems to me like it might work. If that sounds like a reasonable option, I'd be happy to update this PR when I have time.

saem commented 3 years ago

For those discovering this much later, the reason why this hasn't been accepted or moved forward is because it runs counter to how Nim tooling works. Note, I don't understand all the details and am new to Nim and the extension's code (learning bits and pieces as I port it).

The way I've seen Nim work is that the file provided to Nim, nimsiggest, etc... Are an entry point akin to say webpack or JavaScript bundler entry point. From there imports describe the modules that need to be in scope for consideration, be it compiling or providing suggestions.

I believe @kosz78 is alluding to that above when they say each file starts a nimSuggest process. I believe this PR should be closed or changed to improving the docs for the setting and/or changing the name so it's clearer as to the intention of the existing setting, something like project entry point files. Would make a great easy first change for someone.

I realize because many modules will act as a library or a main module flip to tests or some sort of simple driver, that should be handled as a separate matter which I believe is covered by "run this file". The features that might be desirable in that space are likely pattern based defines/settings/etc that are passed to nimSuggest or what not so it does the right thing and can keep a smaller pool of processes in perhaps an LRU fashion so as to not burden the system and still provide the features people desire.

The above would also cover usecase where nimSuggest should respect defines such as is when, like myself, and working on a strictly is target project.

RSDuck commented 3 years ago

the intended way to use nimsuggest is to only have a single instances. To allow this nimsuggest may be started without a project file and tries to give results to files unrelated to the current project, though this was implemented before nimsuggest support was initially implemented in this extension. As a workaround the infamous spawning one nimsuggest instance per file was implemented, which was kept till today.