Skip to content

Add openRecent query parameter #272

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

Merged
merged 5 commits into from
May 21, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -31,6 +31,28 @@ The `vscodessh` subcommand on the `coder` binary periodically flushes it's netwo

Coder Remote periodically reads the `network-info-dir + "/" + matchingSSHPID` file to display network information.

## Testing
Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted this section below the development section, but when I do that Prettier goes haywire and starts injecting spaces all over the place, leading to insanity like this:

- Use the "VS Code Desktop" button from a Coder dashboard.
- Manually    open the link wit   h `Developer: Ope   n URL` from in
    sid   e VS Code.
    - Use    `code --    open-url    <
    link    >` on the     co   mm    and l    ine.

The        
        link format i    s `        v   sc    ode://co    der      
      .cod   er-    r        emote/    ope    n?$        {   qu    ery}
    `        .             For exam               ple:        
    
``     
       `bash
    cod               e                 --ope    n        -url    
             'vs            c           od    e        :/                /coder    
    .        cod                    e            r           -r    e        mo         
           te/ope        n                                            ?u        rl=               d                ev.cod    e                                    r            .c        om            &own                                   e        r=        my        -    u    s                    e        rn    am        e    &w                o               r    k    s                        pa    c    e                    =my    -                ws&    a            ge                                                                           nt=my-agent'

Copy link
Member

@Parkreiner Parkreiner May 20, 2024

Choose a reason for hiding this comment

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

I think all documentation should look like that. Make everything like House of Leaves


There are a few ways you can test the "Open in VS Code" flow:

- Use the "VS Code Desktop" button from a Coder dashboard.
- Manually open the link with `Developer: Open URL` from inside VS Code.
- Use `code --open-url` on the command line.

The link format is `vscode://coder.coder-remote/open?${query}`. For example:

```bash
code --open-url 'vscode://coder.coder-remote/open?url=dev.coder.com&owner=my-username&workspace=my-ws&agent=my-agent'
```

There are some unit tests as well:

```bash
yarn test
```

However, fully testing the plugin is blocked on switching back to the standard VS Code extension testing framework.

## Development

1. Run `yarn watch` in the background.
48 changes: 25 additions & 23 deletions src/commands.ts
Original file line number Diff line number Diff line change
@@ -223,6 +223,7 @@ export class Commands {
treeItem.workspaceName,
treeItem.workspaceAgent,
treeItem.workspaceFolderPath,
true,
)
}
}
@@ -232,6 +233,7 @@ export class Commands {
let workspaceName: string
let workspaceAgent: string | undefined
let folderPath: string | undefined
let openRecent: boolean | undefined

if (args.length === 0) {
const quickPick = vscode.window.createQuickPick()
@@ -340,9 +342,10 @@ export class Commands {
workspaceName = args[1] as string
// workspaceAgent is reserved for args[2], but multiple agents aren't supported yet.
folderPath = args[3] as string | undefined
openRecent = args[4] as boolean | undefined
}

await openWorkspace(workspaceOwner, workspaceName, workspaceAgent, folderPath)
await openWorkspace(workspaceOwner, workspaceName, workspaceAgent, folderPath, openRecent)
}

public async updateWorkspace(): Promise<void> {
@@ -369,6 +372,7 @@ async function openWorkspace(
workspaceName: string,
workspaceAgent: string | undefined,
folderPath: string | undefined,
openRecent: boolean | undefined,
Comment on lines 373 to +375
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for not using optional parameters via ?:?

Copy link
Member Author

@code-asher code-asher May 20, 2024

Choose a reason for hiding this comment

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

Just to make sure it is specified. I find sometimes when I add new arguments, if I make it optional with ?: I can miss calls where I should have added it. But doing it this way forces me to check every function call since they will error without being passed a value.

Once that is done, I might change it to ?: after some consideration, but in this case since in all our calls to this function we have a value to pass, I did not bother.

Copy link
Member

@Parkreiner Parkreiner May 21, 2024

Choose a reason for hiding this comment

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

Yeah, that makes sense. I think that I've favored the optional syntax for my own convenience up 'til now, but I can see how forcing all the parameters to be explicit would be handy, especially for code stability

) {
// A workspace can have multiple agents, but that's handled
// when opening a workspace unless explicitly specified.
@@ -383,36 +387,34 @@ async function openWorkspace(
newWindow = false
}

// If a folder isn't specified, we can try to open a recently opened folder.
if (!folderPath) {
// If a folder isn't specified or we have been asked to open the most recent,
// we can try to open a recently opened folder/workspace.
if (!folderPath || openRecent) {
const output: {
workspaces: { folderUri: vscode.Uri; remoteAuthority: string }[]
} = await vscode.commands.executeCommand("_workbench.getRecentlyOpened")
const opened = output.workspaces.filter(
// Filter out `/` since that's added below.
// Remove recents that do not belong to this connection. The remote
// authority maps to a workspace or workspace/agent combination (using the
// SSH host name). This means, at the moment, you can have a different
// set of recents for a workspace versus workspace/agent combination, even
// if that agent is the default for the workspace.
Comment on lines +397 to +401
Copy link
Member

Choose a reason for hiding this comment

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

Do we have any docs on how this works? The comment seems well-written, but I feel like I'm still not quite getting it because of missing context

Copy link
Member Author

@code-asher code-asher May 20, 2024

Choose a reason for hiding this comment

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

Nope, not that I know of. I am honestly not really sure how to even write the comment, I rewrote it several times and still feel like I am not really explaining it well.

The problem is that both of these go to the same place:

ssh coder-vscode--my-workspace.my-agent
ssh coder-vscode--my-workspace # opens my-agent agent because my-agent is the default or only one

But VS Code has no idea these are the same, they are different strings after all. So depending on which you use you could get a different set of recents.

When clicking from the dashboard we seem to use the second form (without the agent name).

I think probably what we should do is always append the agent name if it is missing, and never use the workspace-only form. Then this will not be a problem at all. But for now I just left the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

And to further clarify, this only arises when using the sidebar in VS Code, where you can both connect to a workspace or directly to an agent.

(opened) => opened.folderUri?.authority === remoteAuthority,
)
if (opened.length > 0) {
let selected: (typeof opened)[0]

if (opened.length > 1) {
const items: vscode.QuickPickItem[] = opened.map((folder): vscode.QuickPickItem => {
return {
label: folder.folderUri.path,
}
})
const item = await vscode.window.showQuickPick(items, {
title: "Select a recently opened folder",
})
if (!item) {
return
}
selected = opened[items.indexOf(item)]
} else {
selected = opened[0]
// openRecent will always use the most recent. Otherwise, if there are
// multiple we ask the user which to use.
if (opened.length === 1 || (opened.length > 1 && openRecent)) {
folderPath = opened[0].folderUri.path
} else if (opened.length > 1) {
const items = opened.map((f) => f.folderUri.path)
folderPath = await vscode.window.showQuickPick(items, {
title: "Select a recently opened folder",
})
if (!folderPath) {
// User aborted.
return
}

folderPath = selected.folderUri.path
}
}

5 changes: 4 additions & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
@@ -140,6 +140,9 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
const workspace = params.get("workspace")
const agent = params.get("agent")
const folder = params.get("folder")
const openRecent =
params.has("openRecent") && (!params.get("openRecent") || params.get("openRecent") === "true")

if (!owner) {
throw new Error("owner must be specified as a query parameter")
}
@@ -166,7 +169,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
await storage.setSessionToken(token)
}

vscode.commands.executeCommand("coder.open", owner, workspace, agent, folder)
vscode.commands.executeCommand("coder.open", owner, workspace, agent, folder, openRecent)
}
},
})