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
Share VS Code settings/extensions nicely #57671
Conversation
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
1 similar comment
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
ah wait, I have an idea for how to solve the |
it only builds on this and might have some drawbacks, though, so I'll do that in a followup PR if this one is merged |
.vs | ||
/.vscode/* | ||
!/.vscode/extensions.json |
git clean -xfd
would still blow away vscode settings right? Maybe we should encourage people to use python setup.py clean
instead
it already does that even before this PR but yeah, that sounds like a good idea to me; in that case could you open a PR to replace git clean -xfd
with python setup.py clean
in CONTRIBUTING.md
?
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: This is a second attempt at pytorch#51214. It should achieve the same goals with (as far as I can tell) no disadvantages, but the advantages are a bit less pronounced than in the more dictatorial approach that pytorch#51214 took: - Unfortunately, I was unable to figure out how to include [the `mypy` configuration given in the docstring of `tools.mypy_wrapper.main`](https://github.com/pytorch/pytorch/blob/7115a4b8707115b81417481fb6240617ae934806/tools/mypy_wrapper.py#L81-L89), because as walterddr pointed out, `"${env:HOME}/miniconda3/envs/pytorch/bin/python"` is not guaranteed to be correct on everyone's machine: ```json { "python.linting.enabled": true, "python.linting.mypyEnabled": true, "python.linting.mypyPath": "${env:HOME}/miniconda3/envs/pytorch/bin/python", "python.linting.mypyArgs": [ "${workspaceFolder}/tools/mypy_wrapper.py" ] } ``` Importantly, this does not work: ```json "python.linting.mypyPath": "${workspaceFolder}/tools/mypy_wrapper.py" ``` This is because VS Code does not run the given `mypy` command inside of the user's specified virtual environment, so for instance, on my system, setting the `mypy` command to directly call `tools/mypy_wrapper.py` results in using `mypy 0.782` instead of the correct `mypy 0.812`. Sadly, [this](https://code.visualstudio.com/docs/editor/variables-reference#_configuration-variables) does not work either, although I'm not sure why: ```json { "python.linting.mypyPath": "${config:python.pythonPath}", "python.linting.mypyArgs": [ "${workspaceFolder}/tools/mypy_wrapper.py" ] } ``` - As a result, `git clean -fdx; tools/vscode_settings.py` still results in some loss of useful configuration. One other thing to note: as `.vscode/settings_recommended.json` shows, there are some configuration sections that only take effect within the context of a `"[language]"`, so currently, if a dev already has one of those settings, it would be entirely overwritten by `tools/vscode_settings.py` rather than a graceful merge. This could probably be fixed by using a deep merge instead of the current shallow merge strategy. Pull Request resolved: pytorch#57671 Test Plan: If you want, you can typecheck the small script added by this PR (no output is expected): ```sh tools/mypy_wrapper.py $PWD/tools/vscode_settings.py ``` You can also try running it to update your own VS Code workspace settings: ```sh tools/vscode_settings.py ``` This should have minimal impact on your existing `tools/settings.json` file other than enabling the few explicitly recommended settings (e.g. it should not reorder or remove any of your existing settings). Reviewed By: malfet Differential Revision: D28230390 Pulled By: samestep fbshipit-source-id: 53a7907229e5807c77531cae4f9ab9d469fd7684
This is a second attempt at #51214. It should achieve the same goals with (as far as I can tell) no disadvantages, but the advantages are a bit less pronounced than in the more dictatorial approach that #51214 took:
Unfortunately, I was unable to figure out how to include the
mypy
configuration given in the docstring oftools.mypy_wrapper.main
, because as @walterddr pointed out,"${env:HOME}/miniconda3/envs/pytorch/bin/python"
is not guaranteed to be correct on everyone's machine:Importantly, this does not work:
This is because VS Code does not run the given
mypy
command inside of the user's specified virtual environment, so for instance, on my system, setting themypy
command to directly calltools/mypy_wrapper.py
results in usingmypy 0.782
instead of the correctmypy 0.812
.Sadly, this does not work either, although I'm not sure why:
As a result,
git clean -fdx; tools/vscode_settings.py
still results in some loss of useful configuration.One other thing to note: as
.vscode/settings_recommended.json
shows, there are some configuration sections that only take effect within the context of a"[language]"
, so currently, if a dev already has one of those settings, it would be entirely overwritten bytools/vscode_settings.py
rather than a graceful merge. This could probably be fixed by using a deep merge instead of the current shallow merge strategy.Test plan:
If you want, you can typecheck the small script added by this PR (no output is expected):
tools/mypy_wrapper.py $PWD/tools/vscode_settings.py
You can also try running it to update your own VS Code workspace settings:
This should have minimal impact on your existing
tools/settings.json
file other than enabling the few explicitly recommended settings (e.g. it should not reorder or remove any of your existing settings).The text was updated successfully, but these errors were encountered: