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

Fix keyword docstring #941

Merged
merged 1 commit into from
Jul 13, 2017
Merged

Conversation

micbou
Copy link
Contributor

@micbou micbou commented Jul 12, 2017

Jedi raises a NotImplementedError exception when asking for the docstring of any Python keyword. For instance, running this code with latest dev branch

import jedi

jedi.Script('assert').completions()[0].docstring()

outputs

Traceback (most recent call last):
  File "jedi\jedi\cache.py", line 119, in wrapper
    return dct[key]
KeyError: ((), frozenset({('fast', False)}))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test.py", line 10, in <module>
    jedi.Script('assert').completions()[ 0 ].docstring()
  File "jedi\jedi\api\classes.py", line 472, in docstring
    return super(Completion, self).docstring(raw=raw, fast=fast)
  File "jedi\jedi\api\classes.py", line 248, in docstring
    return _Help(self._name).docstring(fast=fast, raw=raw)
  File "jedi\jedi\api\classes.py", line 723, in docstring
    for context in self._get_contexts(fast=fast):
  File "jedi\jedi\cache.py", line 121, in wrapper
    result = method(self, *args, **kwargs)
  File "jedi\jedi\api\classes.py", line 713, in _get_contexts
    return self._name.infer()
  File "jedi\jedi\evaluate\filters.py", line 21, in infer
    raise NotImplementedError
NotImplementedError

This is fixed by implementing the infer method in KeywordName and by replacing the docstr property (which is used nowhere) with the py__doc__ method in Keyword. imitate_pydoc only accepts a string so we make sure that we pass one and not a KeywordName object. Also, we strip the trailing \n characters from the docs.

I am not completely sure about the implementation so tell me if things should be done differently and I'll update the PR. In particular, I am wondering if Keyword should be a subclass of Context since they have api_type and py__doc__ in common.

Added a test and replaced raw_doc by docstring() in the docstring tests as the former is deprecated in favor of the latter according to the docs.


This change is Reviewable

@davidhalter
Copy link
Owner

This pull request looks definitely very good, thank you!

In general I would think that everything should be a Context. However, I'm not sure if that makes Context a more complex object than it already is.

I'm merging. If you're interested in diving into the details of if Keywords could be Contexts, I would very much appreciate it (if it's not possible, please also tell me).

Thanks a lot anyways :)

@davidhalter davidhalter merged commit f524825 into davidhalter:dev Jul 13, 2017
@micbou
Copy link
Contributor Author

micbou commented Jul 13, 2017

If you're interested in diving into the details of if Keywords could be Contexts, I would very much appreciate it (if it's not possible, please also tell me).

I'll look into it.

Thanks for merging.

@micbou micbou deleted the fix-keyword-docstring branch July 14, 2017 00:27
zzbot added a commit to vheon/JediHTTP that referenced this pull request Aug 20, 2017
[READY] Update Jedi to latest commit

Jedi master branch now includes davidhalter/jedi#941 and davidhalter/jedi#942 fixes, and successfully passes ycmd tests.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/vheon/jedihttp/36)
<!-- Reviewable:end -->
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.

2 participants