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

Fix the layers dependencies which required a lsp backend #16754

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

Conversation

sunlin7
Copy link
Contributor

@sunlin7 sunlin7 commented Dec 26, 2024

Fix the layers dependency which required a lsp backend.
These layers configuration indicate user want lsp feature, so the layer should depends on the lsp.

@bcc32
Copy link
Collaborator

bcc32 commented Dec 27, 2024

The dap layer depends on lsp already. If I understand correctly, users who do use dap features would see breaking behavior after this change.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Dec 27, 2024

@bcc32 Hidden logic here for the user who want lsp backend but the layer depends on dap, just because of dap layer had requested the lsp layer.

This patch corrects the dependencies relations. (the dap depends on lsp from its implementation, but the lsp should not depends on dap for the dap will affect the user experience a lot, the dap is heavy and under developing).

The dap layer should be explicitly configured in dot spacemacs file if user really use it.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Dec 27, 2024

It's a bug, not a feature, for the layers who actually should depend on the lsp but currently depends on the dap works for user.

@bcc32
Copy link
Collaborator

bcc32 commented Dec 29, 2024

It's a bug, not a feature, for the layers who actually should depend on the lsp but currently depends on the dap works for user.

No, this is a deliberate feature. I suppose in theory it could specify both dap and lsp as dependencies, but it is intentional that enabling go layer with go-backend set to lsp will implicitly activate `dap layer. See this commit for an example of the historical context.

dap will affect the user experience a lot, the dap is heavy and under developing).

If you want a way to opt out of dap while still using lsp for go-backend, maybe we should just add a new config variable instead of piggy-backing on go-backend.

My objection is only that we should try to stay backwards compatible with existing user's configs, and the proposed change is currently incompatible.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Dec 29, 2024

If you want a way to opt out of dap while still using lsp for go-backend, maybe we should just add a new config variable instead of piggy-backing on go-backend.

That's what I want, I used the lsp daily, but never use the dap, and the dap make Spacemacs slow on opening a simple *.py or *.c file.

@bcc32
Copy link
Collaborator

bcc32 commented Dec 29, 2024

Makes sense, let's add a new variable then.

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