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

Workspace folder picker command (for #32936) #34648

Merged
merged 3 commits into from
Sep 20, 2017
Merged

Workspace folder picker command (for #32936) #34648

merged 3 commits into from
Sep 20, 2017

Conversation

bpasero
Copy link
Member

@bpasero bpasero commented Sep 19, 2017

fixes #32936

Introduces a simple new command vscode.pickWorkspace to pick a WorkspaceFolder via quick pick.

@bpasero
Copy link
Member Author

bpasero commented Sep 19, 2017

@jrieken

What does this return? It must be triple-equal to the things we have in workspace.workspaceFolders. Also, why not have this as real API? Having the logic in the core shouldn't mean we cannot have a function for this

good point about the return type and identity, that is addressed in this change.

I rethought about having real API for this because I somewhat assume that this is a corner case where people might need it. If we want to go away from introducing more commands, I can also restore the proper API again and call into the command.

@jrieken
Copy link
Member

jrieken commented Sep 20, 2017

that this is a corner case where people might need it.

Fair enough, I let @dbaeumer decide if a command is OK for him

@jrieken
Copy link
Member

jrieken commented Sep 20, 2017

Oh, the test actually doesn't pass: https://travis-ci.org/Microsoft/vscode/jobs/277395186#L4159

@dbaeumer
Copy link
Member

I always prefer explicit API over commands. The reason is:

  • easier to discover
  • I have code complete
  • one place of documentation
  • easier to keep compatible since we have compilers that check signatures.

But that might be me :-)

@bpasero
Copy link
Member Author

bpasero commented Sep 20, 2017

@jrieken @dbaeumer fair enough, I provide this API now (while still using the command internally):

/**
 * Shows a selection list of [workspace folders](#workspace.workspaceFolders) to pick from.
 * Returns `undefined` if no folder is open.
 *
 * @param options Configures the behavior of the workspace folder list.
 * @return A promise that resolves to the workspace folder or `undefined`.
 */
export function showWorkspaceFolderPick(options?: WorkspaceFolderPickOptions): Thenable<WorkspaceFolder | undefined>;

/**
 * Options to configure the behaviour of the [workspace folder](#WorkspaceFolder) pick UI.
 */
export interface WorkspaceFolderPickOptions {

	/**
	 * An optional string to show as place holder in the input box to guide the user what to pick on.
	 */
	placeHolder?: string;

	/**
	 * Set to `true` to keep the picker open when focus moves to another part of the editor or to another window.
	 */
	ignoreFocusOut?: boolean;
}

/**
* Options to configure the behaviour of the [workspace folder](#WorkspaceFolder) pick UI.
*/
export interface WorkspaceFolderPickOptions {
Copy link
Member

Choose a reason for hiding this comment

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

Don't define types inside a namespace but globally

Copy link
Member Author

Choose a reason for hiding this comment

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

@jrieken isn't that globally? It is outside the window namespace.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry. Didn't see that closing-curly

@bpasero bpasero merged commit 492ae2a into master Sep 20, 2017
@bpasero bpasero deleted the ben/pr-34637 branch September 20, 2017 13:06
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We should have a workspace folder picker API in the Extension host
3 participants