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.

Sign up for GitHub

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

Jump to bottom

Improve module loading path handling at completion, liinting, and so on #712

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Improve module loading path handling at completion, liinting, and so on #712

wants to merge 10 commits into from

Commits on Jan 17, 2020

  1. Introduce config source for "pyls"

    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    25e2773 View commit details
    Browse the repository at this point in the history

  2. Add utility to access configuration value by path 

    At getting the value in nested settings dict, newly added
get_config_by_path() is more efficient and readable than "get with
empty dict" chain like below, which is sometimes used in current
implementation.

  settings.get("foo", {}).get("bar", {}).get("baz")
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    1d3141d View commit details
    Browse the repository at this point in the history

  3. Introduce pyls.source_roots configuration 

    Before this commit, 'setup.py' or 'pyproject.toml' should be created
to specify source root(s) other than the workspace root, even if such
file is not actually needed.

In order to explicitly specify source root(s) in the workspace, this
commit introduces pyls.source_roots configuration, and makes
workspace.source_roots() return its value if configured.

Path in pyls.source_roots is ignored, if it refers outside of the
workspace.

This configuration is also useful in the case that the workspace
consists of multiple (independent) services and a library shared by
them, for example. In such case, N + 1 source roots are listed in
pyls.source_roots configuration.

BTW, this commit puts some utilities into _util.py for reuse in the
future. Especially, os.path.commonprefix() for checking inside-ness in
find_parents() below should be replaced with is_inside_of(), because
the former returns unintentional value in some cases.

    if not os.path.commonprefix((root, path)):
        log.warning("Path %s not in %s", path, root)
        return []

For example:

  - commonprefix(('/foo', '/bar')) returns not false value but '/'
  - commonprefix(('/foo', '/foobar')) returns not false value but '/foo'
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    b28c544 View commit details
    Browse the repository at this point in the history

  4. Intorduce ConfigSource.lsp_config to hook at updating

    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    bd23fcf View commit details
    Browse the repository at this point in the history

  5. Normalize ply.source_roots at LSP didChangeConfiguration method 

    This is useful to specify source roots in relative path, if pyls
process might run in other than the root of the target workspace(s).
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    6dd3149 View commit details
    Browse the repository at this point in the history

  6. Move plugins.jedi.extra_paths handling into Document.sys_path() 

    Before this commit, plugins.jedi.extra_paths configuration is used
only at execution of jedi.Script in Document.jedi_script().

Therefore, at spawning a process for other plugins (e.g pylint), such
extra paths are ignored. It might causes unintentional failure of
finding out libraries installed into those locations, even though Jedi
can find out.

After this commit, just using Document.sys_path() instead of sys.path
is enough path configuration for any plugins.
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    8094a09 View commit details
    Browse the repository at this point in the history

  7. Read plugins.jedi.extra_paths configuration also from files 

    Before this commit, plugins.jedi.extra_paths can be configured only
via Language Server Protocol didChangeConfiguration method.

This is inconvenient, if:

  - it is difficult to issue LSP didChangeConfiguration method with
    per-workspace configuration

  - such configuration should be persisted inside workspace

This commit reads plugins.jedi.extra_paths configuration also from
project-level configuration "setup.cfg" and "tox.ini".
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    ff1a6c2 View commit details
    Browse the repository at this point in the history

  8. Extract py_run of pylint.epylint as spawn_pylint 

    This is the preparation for spawning pylint process with pyls specific
customization in subsequent change.

Almost all of spawn_pylint is cited from pylint/epylint.py in
6d818e3b84b853fe06f4199eb8408fe6dfe033be (the latest version on master
branch at 2019-12-04) of pylint.

BTW, this commit imports StringIO manually for compatibility between
Python 2.x and 3.x instead of using "six" like recent pylint 1.9.x
(supporting python 2.x), because requiring "six" only for StringIO
seems over-kill.
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    5aee574 View commit details
    Browse the repository at this point in the history

  9. Make pylint respect libraries installed into extra paths 

    For example, jedi respects VIRTUAL_ENV environment variable at finding
out libraries. Therefore, (virtualenv) runtime for pyls/jedi can be
separated from one for the target workspace.

On the other hand, pylint does not respect VIRTUAL_ENV, and might
cause unintentional "import-error" (E0401) for libraries installed in
such virtualenv, even though jedi can recognize them.

In order to make pylint respect libraries installed into extra
paths, this commit uses Document.sys_path() instead of sys.path of
current pyls process, at spawning pylint.

At this commit, Document.sys_path() should respect source roots in the
workspace, VIRTUAL_ENV, PYTHONPATH, and plugins.jedi.extra_paths
configuration.
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    5a4b780 View commit details
    Browse the repository at this point in the history

  10. Add description about configuration in README.rst 

    This commit describes about not only features added by previous
commits but also some underlying specifications.
    @flying-foozy
    flying-foozy committed Jan 17, 2020
    Configuration menu
    Copy the full SHA
    3715b28 View commit details
    Browse the repository at this point in the history