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

Refactor inspect info reply logic #13863

Merged
merged 6 commits into from
Dec 21, 2022
Merged

Conversation

jasongrout
Copy link
Member

@jasongrout jasongrout commented Dec 9, 2022

This refactors the logic of the inspector _get_info function to make it easier to override and extend its behavior. This PR should not affect the overall function of _get_info.

We factor out the logic to append an info field to the inspect_reply mimebundle and the logic for what information is in the mimebundle into separate functions that can easily be overridden by an inspector subclass. This allows a subclass to easily:

  • format information into yet another mimetype besides text/plain and text/html
  • modify or add to the default information

without having to copy the default implementation.

We also refactor and simplify the logic around formatting text/plain and text/html mimetypes in the inspect reply.

We also fix an escaping bug with the default html formatter so that the default docstrings <no docstring> appears instead of being interpreted as an HTML tag.

… behavior.

This factors out the logic to append an info field to the inspect_reply mimebundle and the logic for what information is in the mimebundle into separate functions that can easily be overridden by an inspector subclass. This allows a subclass to easily:

* format information into yet another mimetype besides text/plain and text/html
* modify or add to the default information

without having to copy the default implementation.
Also simplify the formatting of the inspect reply text/plain logic
Otherwise, the default docstring <no docstring> was not displayed in html because it was acting as an html tag.
@krassowski
Copy link
Member

krassowski commented Dec 9, 2022

I was recently looking at these very lines and thinking about making the mime bundle more extendable in python-lsp/docstring-to-markdown#25 (comment). I thought it's worth a mention in case if you find that discussion useful.

@jasongrout
Copy link
Member Author

Nice, thanks for pointing to your other discussion there. Do you think these changes are enough to experiment with the ideas you proposed in that other comment? (I think probably they are?). Also #13865 is important to make it easy for a user to change the inspector from configuration or command line, so they can easily use a subclass that is distributed separately.

@Carreau Carreau merged commit d8067af into ipython:main Dec 21, 2022
@Carreau Carreau added this to the 8.8 milestone Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants