-
Notifications
You must be signed in to change notification settings - Fork 23
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
Changes from all commits
96b8229
d7d1b82
9a3734d
28e11df
26c58ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any particular reason for not using optional parameters via There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Once that is done, I might change it to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
} | ||
|
||
|
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.
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:
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.
I think all documentation should look like that. Make everything like House of Leaves