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

[python/pyre.vim] Fix pyre persistent behavior #3895

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

oliverralbertini
Copy link
Contributor

@oliverralbertini oliverralbertini commented Sep 8, 2021

It's necessary to provide a -l option to pyre with the closest parent
directory containing a .pyre_configuration.local file, or simply
change directory (cwd) to the root of the pyre project. Thanks to Ken
Verbosky for the code that fixes this.

Error seen when not using such a solution:

1031.473923 on 6: Dropping message 'ƛ Background task unexpectedly quited: Invalid configuration: Cannot find any source files to analyze. Either `source_directories` or `targets` must be specified.

Issue with this approach is that if you are editing files under
different projects, the pyre persistent process is not re-created for
each file. We have to do :ALEStopAlllsps in order for the process to
start with the new working directory.

@oliverralbertini oliverralbertini force-pushed the fix_pyre_persistent branch 2 times, most recently from e0992b0 to 4380887 Compare September 9, 2021 04:03
@oliverralbertini oliverralbertini changed the title Fix pyre persistent behavior with -l <project> flag [python/pyre.vim] Fix pyre persistent behavior Sep 9, 2021
@oliverralbertini
Copy link
Contributor Author

I'm seeing a couple of failures, but I'm not sure they're related to my change:

[00:01:44]     (5/6) [EXECUTE] (X) {'line_length': 17, 'conn_id': 347, 'source': 'ale-import', 'column': 7, 'request_id': 1, 'line': 2, 'additional_edits_only': 1, 'completion_filter': function('ale#completion#python#CompletionItemFilter'), 'prefix': 'missingword'} should be equal to {'request_id': 1, 'conn_id': 347, 'source': 'ale-import', 'column': 7, 'line_length': 17, 'line': 2, 'additional_edits_only': 1, 'completion_filter': 'ale#completion#python#CompletionItemFilter', 'prefix': 'missingword'}
[00:01:44]     (6/6) [  GIVEN] Some example Python code
[00:01:44]     (6/6) [EXECUTE] ALEImport should tell the user when no completions were found from a language server
[00:01:44]     (6/6) [EXECUTE] (X) {'line_length': 17, 'conn_id': 347, 'source': 'ale-import', 'column': 7, 'request_id': 1, 'line': 2, 'additional_edits_only': 1, 'completion_filter': function('ale#completion#python#CompletionItemFilter'), 'prefix': 'missingword'} should be equal to {'request_id': 1, 'conn_id': 347, 'source': 'ale-import', 'column': 7, 'line_length': 17, 'line': 2, 'additional_edits_only': 1, 'completion_filter': 'ale#completion#python#CompletionItemFilter', 'prefix': 'missingword'}

@hsanson
Copy link
Contributor

hsanson commented Sep 9, 2021

@oliverralbertini you changed the completion_filter value in the linter definition from ale#completion#python#CompletionItemFilter to function('ale#completion#python#CompletionItemFilter') and that is why the test is failing.

Revert the change or update the tests with the new value, whichever makes the linter work properly.

@oliverralbertini
Copy link
Contributor Author

@oliverralbertini you changed the completion_filter value in the linter definition from ale#completion#python#CompletionItemFilter to function('ale#completion#python#CompletionItemFilter') and that is why the test is failing.

Thanks @hsanson, great catch. I will just revert it, seems from the docs that it can be either a string of funcref:

  `completion_filter`     A |String| or |Funcref| for a callback function
                         accepting a buffer number and a completion item.

                         The completion item will be a |Dictionary| following
                         the Language Server Protocol `CompletionItem`
                         interface as described in the specification,
                         available online here:
                         https://microsoft.github.io/language-server-protocol

It's necessary to provide a `-l` option to pyre with the closest parent
directory containing a `.pyre_configuration.local` file, or simply
change directory (cwd) to the root of the pyre project. Thanks to Ken
Verbosky for the code that fixes this.

Error seen when not using such a solution:

```
1031.473923 on 6: Dropping message 'ƛ Background task unexpectedly quited: Invalid configuration: Cannot find any source files to analyze. Either `source_directories` or `targets` must be specified.
```

Issue with this approach is that if you are editing files under
different projects, the `pyre persistent` process is not re-created for
each file. We have to do `:ALEStopAlllsps` in order for the process to
start with the new working directory.
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good.

@hsanson hsanson merged commit b504eeb into dense-analysis:master Sep 10, 2021
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

2 participants