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

feat(completions): add textDocument/completion request to LSP + workspace handlers #164

Merged

Conversation

juleswritescode
Copy link
Collaborator

@juleswritescode juleswritescode commented Jan 2, 2025

This PR adds the textDocument/completion capability to the lsp server 🎉

  • added completion to the lsp-server's ServerCapabilities
  • Removed the println statements from the workspace, since they were writing to stdout and thus led to crashes for the lsp-client
  • introduced a couple of tracing macros to the lsp server handlers, changed log levels to "info" for methods that happen infrequently (shutdown, initialize etc.)
  • upgraded the vscode-languageclient to 9.0.1, which made for better log output

The autocompletions aren't stable right now – completions are sometimes gathered for the wrong statement, and sometimes, the corresponding statement isn't even found. I will fix this in a separate PR 👍

Besides, I wasn't able to start the pg_cli lsp-proxy with the VSCode extension. I'll fix this in a separate PR, too.

Copy link
Collaborator

@psteinroe psteinroe left a comment

Choose a reason for hiding this comment

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

a few nits, overall lgtm!

@juleswritescode
Copy link
Collaborator Author

juleswritescode commented Jan 4, 2025

@psteinroe Update: Replacing the VsCode Extension's OutputChannel to a LogOutputChannel persists the logs, which is quite nice...

If you look at the output, you can see that the returned "JSON" matches our println statements.

I guess stdout is sent directly to the client?

Screenshot 2025-01-04 at 10 26 18 Screenshot 2025-01-04 at 10 28 15

@psteinroe
Copy link
Collaborator

@juleswritescode the cli (and the lsp proxy) writes the output also to a logfile. I am not sure it's the same logs as the lsp client but it helps me debugging a lot. You can find the path to the directory in the pg_fs crate. I also have a script that automatically opens the latest log file (rotated every day, new logfile every hour) and subscribes to it. I can send it over later.

Copy link
Collaborator

@psteinroe psteinroe left a comment

Choose a reason for hiding this comment

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

awesome!! left a few comments :)

crates/pg_workspace_new/src/workspace/server.rs Outdated Show resolved Hide resolved
crates/pg_workspace_new/src/workspace/server/document.rs Outdated Show resolved Hide resolved
editors/code/src/main.ts Outdated Show resolved Hide resolved
xtask/src/install.rs Outdated Show resolved Hide resolved
crates/pg_lsp_new/src/main.rs Outdated Show resolved Hide resolved
crates/pg_lsp_new/src/server.rs Outdated Show resolved Hide resolved
@juleswritescode
Copy link
Collaborator Author

juleswritescode commented Jan 6, 2025

the cli (and the lsp proxy) writes the output also to a logfile. I am not sure it's the same logs as the lsp client but it helps me debugging a lot. You can find the path to the directory in the pg_fs crate. I also have a script that automatically opens the latest log file (rotated every day, new logfile every hour) and subscribes to it. I can send it over later.

Nice! That's useful. I'm still using the pglsp.log for server logs.
The client logs are different though, those are output by the vscode-languageclient/node#LanguageClient, which was the one throwing the Content-Length error.

Still, please send over the script 🙌

@juleswritescode juleswritescode force-pushed the feat/completions_in_lsp branch from e291189 to 4fb71a5 Compare January 6, 2025 10:19
@juleswritescode
Copy link
Collaborator Author

@psteinroe I cleaned up the PR, wrote a description and it should now be reviewable 👍
I'll address the other issues we talked about in separate PRs.

@juleswritescode juleswritescode marked this pull request as ready for review January 6, 2025 10:28
@juleswritescode juleswritescode force-pushed the feat/completions_in_lsp branch from 4fb71a5 to 3f7c386 Compare January 6, 2025 10:30
@juleswritescode juleswritescode force-pushed the feat/completions_in_lsp branch from 3f7c386 to 528b78b Compare January 6, 2025 10:44
@psteinroe
Copy link
Collaborator

the cli (and the lsp proxy) writes the output also to a logfile. I am not sure it's the same logs as the lsp client but it helps me debugging a lot. You can find the path to the directory in the pg_fs crate. I also have a script that automatically opens the latest log file (rotated every day, new logfile every hour) and subscribes to it. I can send it over later.

Nice! That's useful. I'm still using the pglsp.log for server logs. The client logs are different though, those are output by the vscode-languageclient/node#LanguageClient, which was the one throwing the Content-Length error.

Still, please send over the script 🙌

Do not forget to update the username (psteinroe)!

# Open latest logfile for the lsp
function open_lsp_log() {
  latest_file=$(ls -t /Users/psteinroe/Library/Caches/dev.supabase-community.pglsp/pglsp-logs | head -n 1)
  if [ -n "$latest_file" ]; then
    tail -f "/Users/psteinroe/Library/Caches/dev.supabase-community.pglsp/pglsp-logs/$latest_file"
  else
    echo 'No log files found'
  fi
}

@psteinroe psteinroe merged commit 603a3c9 into supabase-community:main Jan 6, 2025
1 check passed
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