Skip to content

nim: optionally check original file instead of a buffer copy#620

Closed
stefantalpalaru wants to merge 2 commits into
dense-analysis:masterfrom
stefantalpalaru:nim
Closed

nim: optionally check original file instead of a buffer copy#620
stefantalpalaru wants to merge 2 commits into
dense-analysis:masterfrom
stefantalpalaru:nim

Conversation

@stefantalpalaru

Copy link
Copy Markdown

Most Nim projects have a project/file-specific "nim.cfg" which is ignored when running "nim check" on a copy in an unrelated temp dir.

This pull request implements an option to switch between original file and temporary copy checking, defaulting to the former which is guaranteed to always work properly.

@w0rp

w0rp commented Jun 5, 2017

Copy link
Copy Markdown
Member

Does nim support an argument for manually specifying the location of a configuration file? If it does, this can be fixed by setting that configuration file explicitly. See the JSHint linter for an example.

@stefantalpalaru

Copy link
Copy Markdown
Author

Does nim support an argument for manually specifying the location of a configuration file?

No, it doesn't: https://nim-lang.org/docs/nimc.html#compiler-usage-configuration-files

The only arguments is has are for disabling specific steps in the search, not for adding extra paths and filenames.

It also doesn't allow specifying a project directory on the command line. All we can do is copy the whole project in the temp dir (but it would be hard to guess the project dir for files in nested subdirs, duplicating some compiler logic in the process) or just check the original file in its place.

@w0rp

w0rp commented Jun 6, 2017

Copy link
Copy Markdown
Member

In that case, set lint_file to 1 for Nim so it only checks for errors against the file on disk. If we have to work with the file, then do that.

w0rp added a commit that referenced this pull request Jun 6, 2017
@w0rp

w0rp commented Jun 6, 2017

Copy link
Copy Markdown
Member

I have done that now. Now we will only check Nim files on disk, which is the only thing that works well.

@w0rp w0rp closed this Jun 6, 2017
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