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

Handle custom overrides which may not include show_column_numbers or show_error_end settings #79

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

snejus
Copy link

@snejus snejus commented Mar 17, 2024

In projects which configure 'show_column_numbers = true', pylsp would not return anything back to my editor.

In the logs I found messages like below

... discarding result for beetsplug/bandcamp/track.py:113 against /home/sarunas/repo/beetcamp/beetsplug/bandcamp/track.py

The logic only returns the result if the latter filename ends with the former, thus it seems that the column number (':113') is not supposed to be present here.

Having had a look at the matching pattern I found that it does not take into account that the column number may be present.

This commit adjusts the pattern to handle the column number correctly.

Edit:

Seems like the issue was actually caused by me providing custom mypy overrides through the plugin settings which did not include show_column_numbers = true setting that the plugin sets internally.

In projects which configure 'show_column_numbers = true', pylsp would
not return anything back to my editor.

In the logs I found messages like below

```
... discarding result for beetsplug/bandcamp/track.py:113 against /home/sarunas/repo/beetcamp/beetsplug/bandcamp/track.py
```

The logic only returns the result if the latter filename ends with the
former, thus it seems that the column number (':113') is not supposed to
be present here.

Having had a look at the matching pattern I found that it does not take
into account that the column number may be present.

This commit adjusts the pattern to handle the column number correctly.
@Richardk2n
Copy link
Member

Can you provide an example producing this issue?
Given pylsp-mypy sets show-error-end, which implies show_column_numbers, setting that should not make a difference.
This second pattern was introduced for certain errors not honoring these flags and should normally not be used. It seems you stumbled upon one, that partially respects the flag. I was not aware of such errors and would like to investigate.

@snejus
Copy link
Author

snejus commented Apr 26, 2024

Given pylsp-mypy sets show-error-end, which implies show_column_numbers, setting that should not make a difference.

Completely! I later found that this probably happened because I provided my own overrides in the plugin settings which did not include show_column_numbers or show_error_end. I will update the PR description accordingly now.

@snejus snejus changed the title Handle 'show_column_numbers = true' mypy setting Handle 'show_column_numbers = true' mypy setting because of custom overrides Apr 26, 2024
@snejus
Copy link
Author

snejus commented Apr 26, 2024

Given the above, it seems like we'd rather always provide --show-error-end to mypy regardless of whether there are overrides or not. What do you think?

@snejus
Copy link
Author

snejus commented Apr 26, 2024

With overrides = {}

[ERROR][2024-04-26 12:02:20] .../vim/lsp/rpc.lua:734    "rpc"   "/media/pipx-3.8/bin/pylsp"     "stderr"        "2024-04-26 12:02:20,097 BST - WARNING - pylsp_mypy.plugin - executing mypy args = [] via api\n"

With overrides = { true }

[ERROR][2024-04-26 12:03:14] .../vim/lsp/rpc.lua:734    "rpc"   "/media/pipx-3.8/bin/pylsp"     "stderr"        "2024-04-26 12:03:14,040 BST - WARNING - pylsp_mypy.plugin - executing mypy args = ['--show-error-end', '--no-error-summary', '--config-file', '/home/sarunas/repo/pyls-mypy/pyproject.toml', '/home/sarunas/repo/pyls-mypy/pylsp_mypy/plugin.py', '--incremental', '--follow-imports', 'silent'] via api\n"

@snejus snejus changed the title Handle 'show_column_numbers = true' mypy setting because of custom overrides Handle custom overrides which may not include show_column_numbers or show_error_end settings Apr 26, 2024
@snejus
Copy link
Author

snejus commented Apr 26, 2024

See the last three commits:

  • the command-line options used by the plugin are always used - these can be overridden since user overrides are appended and they will take precendence
  • documented the options that the plugin uses
  • removed --no-error-summary as it seems like it's not required

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