abingham / emacs-ycmd

Emacs client for ycmd, the code completion system.
MIT License
384 stars 46 forks source link

company-ycmd: Use new format for python functions in ycmd #431

Closed ptrv closed 7 years ago

ptrv commented 7 years ago

ycmd now indicates a python function in extra_menu_info with def instead of function.

See: https://github.com/Valloric/ycmd/commit/72bfc38f245df319c8dff0965dd86d950b631947

noxdafox commented 7 years ago

I tested the code and it didn't fix the issue. Still see the function definition being suggested as def funcname.

Moreover I guess this change would break backward compatibility as is.

noxdafox commented 7 years ago

Looking a bit further from the logic you pointed out.

Trying the following code:

import requests

def main(foo, bar):
    pass

requests.del      # completing here, expecting completion of delete with params

It seems that the function company-ycmd--extract-params-python receive its first parameter as nil which prevents it from extracting the parameters.

Looking closer, it seems it cannot get the function signature from the requests library. This suggests there is something wrong in my PYTHONPATH variable.

I just tested with Python standard library and the parameters are indeed showing. So it's probably something wrong with my env vars.

Yet it is weird as the semantic completion seems to work, all functions and classes defined in the requests library are suggested. Just the parameters are missing.

ptrv commented 7 years ago

@noxdafox I tried you code snippet and I have the same behavior.

If you run M-x ycmd-display-completions RET you will see that the extra_menu_info only shows def delete without any further information. So this is what we get from ycmd.

The problem before was the parsing of the params itself which the #431 fixes

Maybe worth asking in the ycmd project?

noxdafox commented 7 years ago

Yes I will open an issue against their project. For this patch, IMHO would be better to ensure backward compatibility.

ptrv commented 7 years ago

would be better to ensure backward compatibility.

ycmd is tied to a specific jedi version so no need to maintain backward compatibility

noxdafox commented 7 years ago

Ycmd is enforcing specific version of its dependency right now that it's under heavy development, hopefully in future it will change. Bear in mind the User might still be using old Ycmd.

Debian, for example, now ships a ycmd package: https://packages.debian.org/sid/ycmd

The change would break compatibility with that version for example.

micbou commented 7 years ago

Ycmd is enforcing specific version of its dependency right now that it's under heavy development, hopefully in future it will change.

Not sure why you are saying that. It points to the Jedi release 0.10.2 which is supposed to be stable.

noxdafox commented 7 years ago

What I mean is that I hope to be able to use distribution packages rather than pulling the source code of all the dependencies in future.

https://github.com/Valloric/ycmd/blob/master/.gitmodules

noxdafox commented 7 years ago

@ptrv I tried an older version of ycmd and I get the paramters completion as well (see https://github.com/Valloric/ycmd/issues/773).

EDIT: I forgot to eval the change. Reverting this commit makes company-ycmd showing the function parameters again. So it's definitely something wrong in recent ycmd changes.