Feat/project name variable #3
Conversation
The full test suite is run and passed and the python executable optionnhas now the possibility to use the variable
In the Executable Path option, document the usage of the variable and give an example
|
This solves issue #2 |
lib/init.coffee
Outdated
| resolvePath: (targetPath) -> | ||
| projectPaths = atom.project.getPaths() | ||
| if projectPaths.length > 0 | ||
| projectPath = projectPaths[0] |
There was a problem hiding this comment.
Should not randomly pick the project path at index 0
Should loop over all possible project path and find out which project actually contains the current file being linted (see above comment the file path being linted should be provided as a parameter)
lib/init.coffee
Outdated
| @@ -98,7 +102,7 @@ module.exports = | |||
| @subscriptions = new CompositeDisposable | |||
| @subscriptions.add atom.config.observe 'linter-mypy.executablePath', | |||
| (executablePath) => | |||
| @executablePath = executablePath | |||
| @executablePath = @resolvePath executablePath | |||
There was a problem hiding this comment.
The resolvePath call should not be done immediately after the setting has change/loaded
What if at the time the setting is changed by the user there are more than one project open?, what if no project is open?
I am pretty sure in those scenarios the @executablePath will hold an invalid path which won't change till the user manually edit the setting.
The call to resolve the executable path should instead be done at the last minute Line 249 where the actual call to python is done.
Also the resolving of the path should be done every time a call to python has to be done and therefore when resolving the executable path it should not store permanently the computed result, it should use it to make the call to python and forget about it since the resolved path may be different for the next file to lint.
Note: the resolved path should be use to make the call to python:
AND to be shown in the different pop-up message in case something goes wrong:
There was a problem hiding this comment.
Perfect @elarivie, I will make those changes. :)
| @@ -464,3 +468,12 @@ module.exports = | |||
| else | |||
| # The file is to be ignored, we therefore return an empty set of warning. | |||
| return [] | |||
|
|
|||
| resolvePath: (targetPath) -> | |||
There was a problem hiding this comment.
This method should also need the path to the file being linted, with this path we can then find out which project it belongs to
Parameter#1 = The executable path as provided by the setting
Parameter#2 = The file path of the file being linted
| {CompositeDisposable} = require 'atom' | ||
| helpers = require 'atom-linter' | ||
|
|
||
| fdescribe "The MyPy provider for Linter", -> |
There was a problem hiding this comment.
fdescribe vs describe
There is an extra f at the beginning of the line... is it needed?
The resolvePath function now is 'current file' aware and can get its correct project name even with multiple project roots.
resolvePath is used to call the mypy executable and for the messages given to the user.
|
Now I use the current file for resolvePath(). This way, the method works with multiple project roots. |
lib/init.coffee
Outdated
| return targetPath | ||
|
|
||
| [projectPath, ...] = atom.project.relativizePath(filepath) | ||
| projectName = path.parse(projectPath).name |
There was a problem hiding this comment.
projectPath can be null if the requested filepath is not part of an open Atom project (the file was opened on its own). You should be checking for that before attempting to path.parse it.
This pull request enables the usage of a $PROJECT_NAME variable in the Executable Path option.
With this, the mypy executable of different virtual environments can be used with a single configuration by following the convention of locating all virtual environments in the same directory (e.g. ~/.virtualenvironments) and name them after each project's name.