-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add option to show links to error code docs (once per code) #15449
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like adding yet another option. Maybe this could go in --show-error-context
?
I don't like the idea of adding this functionality to an existing option, at least at first, since this may make error messages quite verbose. This also doesn't seem to fit nicely under the semantics of existing error flags. I noticed that the note is sometimes duplicated:
Also maybe the error code link note should come after other, more specific notes. It seems like something that is only occasionally useful, whereas some other notes are important for understanding the details of an error, so I think that existing notes should be shown first. Maybe we could make the note a bit shorter, without making it less clear? Currently it seems visually quite prominent to me. Random idea: "See https://[...] for more information". I think that optimally, we'd have some shorthand URL for these links, e.g. a trivial forwarding service that would redirect an URL like https://mypy-lang.org/errors/call-overload to the read the docs site (this is a potential future improvement). This would also let us collect statistics on errors users look up the most often. We could maybe consider enabling this by default if the notes would be less verbose. One option would be to not generate them for some common error codes that don't have very interesting information in the documentation, such as perhaps arg-type or the argument count error codes. |
Another option, though questionable how widely supported it is, is to use the hyperlink escape sequence when pretty-printing. |
Yeah as I mentioned in the PR description having a short redirect from mypy-lang.org would be best, and even started looking into it, but the site looks all static, so it was not immediately obvious how to add redirects, so I decided to start with juts the RTD links. Until then we should keep it behind a flag, we can flip the default when we are sufficiently happy with the links. Also yeah, this note definitely should be last if there are other notes, and I will fix duplication. A bit more on motivation for this: I think having these links in the errors are not very useful per se, but it is a good way to advertise their existence, so later one can easily find the docs for given error without going through Google etc. |
This comment has been minimized.
This comment has been minimized.
Diff from mypy_primer, showing the effect of this PR on open source code: graphql-core (https://github.com/graphql-python/graphql-core): typechecking got 1.07x slower (289.9s -> 309.1s)
(Performance measurements are based on a single noisy sample)
|
If there are no more comments, I will merge this later today. Note I am enabling this for self-check, so we can gain some more experience with this. |
A bit late to the party here... I noticed this nice new feature in the Mypy 1.5 Release blog article but I don't see any mention of it in the documentation: https://mypy.readthedocs.io/en/stable/search.html?q=%22--show-error-code-links%22&check_keywords=yes&area=default# Should we add documentation for this flag here? https://mypy.readthedocs.io/en/stable/command_line.html#configuring-error-messages |
PR welcome! |
@asqui Yes, I don't think there is a reason it's not documented, likely I simply forgot to do it. |
Fixes #7186
We can probably add some kind of redirect from mypy-lang.org, but I think RTD link is already OK. This PR will need to wait until next release, unless we want to use
/latest
in the link.