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

Add initial implementation #1

Merged
merged 5 commits into from
Oct 24, 2024
Merged

Add initial implementation #1

merged 5 commits into from
Oct 24, 2024

Conversation

niksy
Copy link
Collaborator

@niksy niksy commented Oct 18, 2024

As per sublimelsp/repository#120 (comment), this PR adds initial implementation for language server.

Waiting on:

Comment on lines +19 to +20
def required_node_version(cls) -> str:
return '>=20'
Copy link
Member

@rchl rchl Oct 18, 2024

Choose a reason for hiding this comment

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

It does not work with v18?
We are currently including 18 with lsp_utils so it would be quite inconvenient if it wouldn't work with that.

(that said, it's probably a good time to update to 20 in lsp_utils)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’m following what is defined in language server package.json engines field: https://github.com/wkillerud/some-sass/blob/main/packages/language-server/package.json#L14

I suppose it could work with Node 18, I know I’ve used it before I switched to Node 20.

Copy link
Member

Choose a reason for hiding this comment

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

Then it's time to update node version in lsp_utils I think.

Copy link
Member

Choose a reason for hiding this comment

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

Doing in sublimelsp/lsp_utils#125.

We should release those at the same time (or lsp_utils first).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’ve update PR description to reflect this.

niksy and others added 2 commits October 20, 2024 11:32
@rchl
Copy link
Member

rchl commented Oct 21, 2024

There is a custom command that the server triggers from code actions after selecting some text: https://github.com/wkillerud/some-sass/blob/0623e9162d70fd526f87b5185459801c9e481b67/vscode-extension/src/client.ts#L170-L234

This would need to be implemented manually in here.
But it's not a blocker for now I guess.

@niksy
Copy link
Collaborator Author

niksy commented Oct 22, 2024

There is a custom command that the server triggers from code actions after selecting some text: wkillerud/some-sass@0623e91/vscode-extension/src/client.ts#L170-L234

This would need to be implemented manually in here. But it's not a blocker for now I guess.

Oh, haven’t seen this one. Is there any previous work in other LSP plugins on how to implement those custom commands?
Should we create issue for this and revisit after initial publish?

@rchl
Copy link
Member

rchl commented Oct 22, 2024

There is a custom command that the server triggers from code actions after selecting some text: wkillerud/some-sass@0623e91/vscode-extension/src/client.ts#L170-L234
This would need to be implemented manually in here. But it's not a blocker for now I guess.

Oh, haven’t seen this one. Is there any previous work in other LSP plugins on how to implement those custom commands? Should we create issue for this and revisit after initial publish?

Existing examples: https://github.com/search?q=org%3Asublimelsp%20on_pre_server_command&type=code

@rchl rchl merged commit 61730fa into sublimelsp:main Oct 24, 2024
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