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

LSP: Fix spec violations that break the VSCode outline #99295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HolonProduction
Copy link
Member

@HolonProduction HolonProduction commented Nov 15, 2024

Fixes #98164

This contains fixes to two situations that would break the vscode outline:

  1. The one described in [LSP] Scripts doesn't begin with extends in the first line shows 'can't find symbols' in the outline #98164:
    As noticed in the issue: with the comment on the first line, the range of the root symbol shifts, which is fine. However, the spec for selectionRange requires the selection range to be inside of the range. When the class has no class_name we would just leave the selection range on its default of the start of the first line. With the root symbol shift this means, that the selection range is outside the range which is a spec violation and breaks the VSCode outline.

Figuring this out was so much more work than the three line fix would make it seem TwT

  1. The conversion between GDScript positions and LSP positions did not take a special case into account where the GDScript column is set to 0 but the line is set 1, for this case it would output an invalid lsp column value of -1 which would break the VSCode outline as well. This case can happen in scripts without extends and class_name statements.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I'm not familiar with the LSP so I'll trust you that this is the correct fix. I assume it was tested on VS Code.

Might be worth testing on e.g. neovim which I believe also has a GDScript LSP implementation. https://github.com/niscolas/nvim-godot

@akien-mga akien-mga added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Nov 22, 2024
@limbonaut
Copy link

I've tested this PR with the limbonaut/limboai#194 issue and Code OSS.
It works well! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:editor topic:gdscript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[LSP] Scripts doesn't begin with extends in the first line shows 'can't find symbols' in the outline
4 participants