Skip to content
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 a dynamic URL to the message documentation in the message descriptions from checkers #9501

Open
mayrholu opened this issue Mar 13, 2024 · 12 comments
Labels
Enhancement ✨ Improvement to a component Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc.

Comments

@mayrholu
Copy link

Question

As the title suggests I am writing a pylint plugin with custom messages. I would like to set a base URL pointing to the html docs of my messages. I cannot find anything in the docs/guides on plugins. Can anyone help? Is there even a way to achieve that?
Thanks in advance!

Documentation for future user

I would have expected it somewhere here:
https://pylint.pycqa.org/en/latest/development_guide/how_tos/custom_checkers.html#defining-a-message

Additional context

No response

@mayrholu mayrholu added Documentation 📗 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Question labels Mar 13, 2024
@DanielNoord
Copy link
Collaborator

Will the plugin be part of the main codebase? Or are you hosting it yourself?

@mayrholu
Copy link
Author

No, it is a custom plugin only intended for use within our company. The html docs will be hosted on our own internal server.

@DanielNoord
Copy link
Collaborator

Then I don't really know how your base URL would work. Are you hosting the pylint docs yourself as well?

@mayrholu
Copy link
Author

No, only the docs for my plugin.

@DanielNoord
Copy link
Collaborator

Then I don't think you can link anywhere. The documentation for each message is autogenerated in our CI/doc build and then uploaded to readthedocs. You can do the same (it's just a make file the in the docs directory) and then host those files somewhere.

There is no way for the publicly available pylint docs to know it should host information about your private plugin.

@mayrholu
Copy link
Author

I thought a solution could be to provide custom URLs directly to the msg-dict:

msgs = {
    'E9999': ('description',
              'message-symbol',
              'Message ....',
              {'custom_url': 'https://custom_url/...'}),
}

Which would mean that the class ExtraMessageOptions would need an extra property custum_url or sth. similiar.

@DanielNoord
Copy link
Collaborator

Can't you put the URL in the message then?

@Pierre-Sassoulas
Copy link
Member

I don't think we want to provide a link to our own documentation in pylint message from the CLI. This would be very verbose and I don't think a lot of person are checking the full message description in the CLI anyway. But in the vs-code pylint plugin we (actually @DudeNr33) hard-coded the url and I think this is bringing a lot of value to users (microsoft/vscode-pylint#298). Making it dynamic would be very nice for pylint plugin integrated in IDE. Reopening for discussion let me know what you think @DanielNoord :)

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection and removed Question Documentation 📗 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Mar 18, 2024
@Pierre-Sassoulas Pierre-Sassoulas changed the title How to set message base URL for plugin Add a dynamic URL to the message documentation in the message descriptions from checkers Mar 18, 2024
@mayrholu
Copy link
Author

@Pierre-Sassoulas Thanks for the hint, I just now realized that this was actually what I was talking about (I am using vscode with pylint-extension). I assumed this was part of pylint.
If there is a way to solve it so the extension shows the correct link I would be very happy. Thanks!

@DanielNoord
Copy link
Collaborator

I mean it would be nice, but I don't really see how it could be so. The VS Code plugin hardcodes the domain currently and doesn't have access to the pylint source code. I'm not quite sure how we will propagate this information and how the plugin should infer that that data is available.

@Pierre-Sassoulas
Copy link
Member

We can add an API by extending the MessageDefinition class with a doc_url attribute for this purpose. Two possibilities, one it's a plugin with a custom url:

msgs = {
    'E9999': ('description',
              'message-symbol',
              'Message ....',
              {'doc_url': 'https://custom_url/...'}),
}

Here the doc_url is going to be https://custom_url/.... Second possibilitie the standard case, where nothing is defined like in all checkers and plugin at the moment:

msgs = {
    'E4999': ('description',
              'message-symbol',
              'Message ....',
}

Here we use the hardcoded version "https://pylint.readthedocs.io/en/latest/user_guide/messages" + what is done in https://github.com/microsoft/vscode-pylint/pull/298/files#diff-6946ea5cbaa105f584c64a7a929c675768847c8e05c86ce965ca8daa18f0ac73R140-R145. (We could even treat plugin/builtin checkers differently).

Then we create a merge request in vscode-pylint where we get the MessageDefinition from the MessageStore (with a default to current behavior in pylint-vscode when has_attr(message_definition, "doc_url") is False so older pylint versions do not crash).

Everything fall into place when we release a version of pylint with the new API.

@Pierre-Sassoulas Pierre-Sassoulas added Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. and removed Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection labels Mar 18, 2024
@DanielNoord
Copy link
Collaborator

It could definitely work but we're not even sure whether the extension would be open to such version dependent functionality.

To me it seems like we can easily make poor api decisions because we just don't really know what sort of edge cases we would need to account for. Since this is the first request for such a feature I would wait to see if there is more demand and whether those demands fall within the proposed api.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc.
Projects
None yet
Development

No branches or pull requests

3 participants