Default pyright settings so LSP doesn't give errors by default#15521
Default pyright settings so LSP doesn't give errors by default#15521mccakit wants to merge 1 commit intomesonbuild:masterfrom
Conversation
|
I don't know what any of that means -- how do these settings correspond to our current mypy settings? |
There was a problem hiding this comment.
Also, I unconditionally oppose adding this if pyright doesn't support starting the config filename with a dot. See how we have a .mypy.ini rather than a mypy.ini
This is important because optional linter settings shouldn't clutter up the main view of the repository by default, when I navigate to the repository in my local development environment, and dotfiles are automatically excluded from said clutter. Most linters do support this, we have a bunch of dotfiles already.
|
Hmm, doc mentions adding configuration settings to pyproject.toml. https://github.com/microsoft/pyright/blob/main/docs/configuration.md#sample-config-file |
Not really. :( I am extremely opposed to the idea that that file should be used for anything other than configuring a PEP 517 build backend. What ends up happening is that people use that file to store information about 15 different programs and then it should really have been named As a practical issue, the problem with an Many of the dotfiles we have, could in theory be "migrated" into pyproject.toml, because the upstream tools in question added support for it, I guess on the theory that lots of people these days prefer using an ... I wonder if this would be a good feature request for pyright? |
|
Well, the problem still needs to be solved at the end of the day. Pyright is the default lsp for many editors, and meson having 270 errors without any modifications is a problem. I recommend just merging this PR, since it is just a config file, overriding is easy and everything can be easily reverted. |
|
I'm okay with making pyright and mypy's behavior match, but we shouldn't turn off warnings that mypy has on. My reasoning is that we have some code that is not yet type safe, but the majority of the code is type safe (with the caveat that we are lacking in nullable type safety). |
|
My changes only disables errors, it doesn't touch warnings. |
This is the same argument that many people use to demand that projects add a vscode directory to ~every repo on GitHub. I don't like this argument at all. Everyone's idea of a "default XXX for most people" is different, making the whole notion of "the default XXX" ring awfully hollow. If your editor is unable to gracefully detect that the repository doesn't include any configuration for pyright, and react accordingly by refraining from emitting "errors" by default when those errors are clearly subjective enough to be worth a configuration option to disable them, then it is a badly designed editor and you should submit a bug report to the editor. An example title for such a bug report is "$EDITOR provides a bad / scary experience by default when opening a GitHub repository whose maintainers don't happen to coincidentally also use $EDITOR". It doesn't feel right to me to penalize every single open source python project in the world because one editor has bad defaults and therefore loads of projects have to override the defaults on the off chance that contributors will be using that editor.
None of this answers the issue of "the mere presence of a filename entry in the repository root decreases the experience of other contributors to the repository". |
|
It is the LSP, and other text editor would have the same errors. It is not an editor thing, but a LSP thing. Pyright just works with defaults without the config. I already made my arguments, you can close the PR if you wish. |
|
To understand better the scope of the changes: the problem with meson is that not all files are typed; but for those that are, you don't want to remove errors/warnings (other than Optional). Am I right that this patch configures out some checks unconditionally, or did I misunderstand it completely? If that is correct, is there a way to configure the LSP to skip some files? Maybe we could reuse that configuration from run_mypy.py too (and even, switch from opt-in to opt-out). |
|
Yes it does unconditionally, there is probably some way to whiteflag/black flag files, but I don't think it would be worth it. |
|
Then this patch is incorrect because the warnings are actually useful for the vast majority of files. Please only limit to warnings that trigger in typed files, for example mesonbuild/compilers/detect.py, mesonbuild/coredata.py or mesonbuild/options.py. |
This commit disables some pyright features so devs can edit meson source without tweaking the lsp. Currently pyright gives 270 errors on zed editor without any changes to lsp settings.