fix: improve openDevContainer support with local workspace folder
#544
Conversation
This change allows `localWorkspaceFolder` and `localConfigFile` to be provided which allows us to use `dev-container` rather than `attached-container` to enter enabling all dev container features like file change detection and rebuilding.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the /openDevContainer command to accept local paths, enabling full dev-container features when a local workspace folder and config file are provided.
- Add
localWorkspaceFolderandlocalConfigFilequery parameters inextension.ts - Extend
Commandsto forward new parameters and adjustopenDevContainerbehavior - Update
CHANGELOG.mdto document the new support
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/extension.ts | Read, validate, and forward localWorkspaceFolder & localConfigFile |
| src/commands.ts | Accept new CLI args, include hostPath/configFile, choose container type |
| CHANGELOG.md | Document unreleased support for hostPath and configFile |
Comments suppressed due to low confidence (1)
src/commands.ts:751
- Add unit or integration tests covering scenarios with and without
localWorkspaceFolderandlocalConfigFileto ensure correct authority strings and request payloads.
async function openDevContainer(
src/commands.ts
Outdated
| const localWorkspaceFolder = args[5] as string | undefined; | ||
| const localConfigFile = args[6] as string | undefined; |
There was a problem hiding this comment.
I know this isn't a new thing you're doing but ???
why does this function not just take named arguments??? I really wish someone had left a justification in a comment.
src/commands.ts
Outdated
| "utf-8", | ||
| ).toString("hex"); | ||
| const devContainerAuthority = `attached-container+${devContainer}@${remoteAuthority}`; | ||
|
|
||
| const type = !!localWorkspaceFolder ? "dev-container" : "attached-container"; |
There was a problem hiding this comment.
| const type = !!localWorkspaceFolder ? "dev-container" : "attached-container"; | |
| const type = localWorkspaceFolder ? "dev-container" : "attached-container"; |
as the linter is saying, this does nothing
There was a problem hiding this comment.
Haha, I fixed this locally but it didn't get pushed. I know it doesn't matter in the context, but it just feels better you know, some habits die hard. 😄
|
tested locally and this diff seems to work fine. could we sneak it in here? diff --git a/src/commands.ts b/src/commands.ts
index c1d49f9..375bca4 100644
--- a/src/commands.ts
+++ b/src/commands.ts
@@ -620,18 +620,18 @@ export class Commands {
*
* Throw if not logged into a deployment.
*/
- public async openDevContainer(...args: string[]): Promise<void> {
+ public async openDevContainer(
+ workspaceOwner: string,
+ workspaceName: string,
+ workspaceAgent: string,
+ devContainerName: string,
+ devContainerFolder: string,
+ ): Promise<void> {
const baseUrl = this.restClient.getAxiosInstance().defaults.baseURL;
if (!baseUrl) {
throw new Error("You are not logged in");
}
- const workspaceOwner = args[0] as string;
- const workspaceName = args[1] as string;
- const workspaceAgent = args[2] as string;
- const devContainerName = args[3] as string;
- const devContainerFolder = args[4] as string;
-
await openDevContainer(
baseUrl,
workspaceOwner,you can add your new arguments to the end as optional |
f79393a to
086ec55
Compare
|
@aslilac made the proposed changes 👍🏻 |
Understandable. 👍🏻 @DanielleMaywood researched this topic in the past and came to the conclusion that These APIs are totally undocumented and have been reverse-engineered. With some additional effort we discovered that More context at: coder/coder#16426 (comment) |
openDevContainer support with local workspace folder
This change allows
localWorkspaceFolderandlocalConfigFileto beprovided which allows us to use
dev-containerrather thanattached-containerto enter enabling all dev container features likefile change detection and rebuilding.
Example open URL: