-
Notifications
You must be signed in to change notification settings - Fork 747
feat(amazonq): remote workspace context #6894
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
Conversation
force use lsp add env launch.json add listeners uri to string uri converter
|
| async function getWorkspaceId(editor: vscode.TextEditor): Promise<string | undefined> { | ||
| try { | ||
| const workspaceIds: { workspaces: { workspaceRoot: string; workspaceId: string }[] } = | ||
| await vscode.commands.executeCommand('aws.amazonq.getWorkspaceId') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a command instead of a plain function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid cyclic dependency. The Flare LSP code was in the packages/amazonq which imports packages/core, but the CW code here is in packages/core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the command isn't defined? Sometimes there are startup ordering issues which result in "command not found" errors. Using a plain function would avoid that and add confidence.
There are more robust ways of avoiding cyclic dependencies. Commands should be a last resort.
1. Add the settings of workspace context. (String of the settings to be adjusted based on product spec) 2. Let inline completion use workspaceId. Before launch date, future workspace context PRs will target branch feature/q-lsp directly. VSC ref: aws/aws-toolkit-vscode#6894
| manifestUrl: 'https://aws-toolkit-language-servers.amazonaws.com/remoteWorkspaceContext/0/manifest.json', | ||
| supportedVersions: '^1.0.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the final URL or will /codewhisperer/ gain the features from the temporary /remoteWorkspaceContext/ artifact (then this version would be 3.x) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is final, to my knowledge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now this is the final URL but I imagine we will need to change the URL in the near future because new artifacts are also being built
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the name is pretty confusing. this defaultAmazonQLspConfig LSP bundle will be used for all Q features, not only "workspace indexing", right?
will this URL also have the upcoming "agentic chat" capabilities?
|
| client.sendNotification('workspace/didCreateFiles', { | ||
| files: e.files.map((it) => { | ||
| return { uri: it.fsPath } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this event trigger for in-memory (non-file) editors? If so, should those be skipped, since the LSP can't do anything with them unless we send the file contents?
| client.info( | ||
| `Client: Updated Amazon Q Profile ${AuthUtil.instance.regionProfileManager.activeRegionProfile?.arn} to Amazon Q LSP`, | ||
| result | ||
| ) | ||
| } catch (err) { | ||
| client.error('Error when setting Q Developer Profile to Amazon Q LSP', err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocker/future reference: these should probably be logged in logger, not client
jpinkney-aws
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging to prepare mainline for ssc
## Problem We did not utilize server side compute to help improve the quality of inline completion, chat, etc. ## Solution 1. Add server side project context support. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
Problem
We did not utilize server side compute to help improve the quality of inline completion, chat, etc.
Solution
feature/xbranches will not be squash-merged at release time.