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.

Sign up for GitHub

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

Jump to bottom

Display prompt to restore all projects #2323

Merged
merged 11 commits into from
Jun 18, 2018
Merged

Display prompt to restore all projects #2323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b613ec5
dotnet restore solution
akshita31 May 9, 2018
9960947
remove using
akshita31 May 9, 2018
1d79866
Perfom restore on the solution
akshita31 May 9, 2018
f624601
use dotnet restore all
akshita31 May 9, 2018
931ca3d
Renamed functions and added return types
akshita31 May 16, 2018
09e4231
remove args from info message
akshita31 May 17, 2018
5610f34
Alter the unresolved dependencies message
akshita31 May 17, 2018
ef58691
Rename to Restore all projects
akshita31 May 18, 2018
ddb85ec
use a single disposable
akshita31 May 29, 2018
73bd180
Merge branch 'master' into restore_solution
Jun 8, 2018
e5a920f
Merge branch 'master' into restore_solution
Jun 13, 2018
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
12 changes: 9 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@
"onCommand:o.restart",
"onCommand:o.pickProjectAndStart",
"onCommand:o.showOutput",
"onCommand:dotnet.restore",
"onCommand:dotnet.restore.project",
"onCommand:dotnet.restore.all",
"onCommand:dotnet.generateAssets",
"onCommand:csharp.downloadDebugger",
"onCommand:csharp.listProcess",
Expand Down Expand Up @@ -606,8 +607,13 @@
"category": ".NET"
},
{
"command": "dotnet.restore",
"title": "Restore Packages",
"command": "dotnet.restore.project",
"title": "Restore Project",
"category": ".NET"
},
{
"command": "dotnet.restore.all",
"title": "Restore All Projects",
"category": ".NET"
},
{
Expand Down
95 changes: 36 additions & 59 deletions src/features/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,33 @@ import CompositeDisposable from '../CompositeDisposable';
import OptionProvider from '../observers/OptionProvider';

export default function registerCommands(server: OmniSharpServer, platformInfo: PlatformInformation, eventStream: EventStream, optionProvider: OptionProvider): CompositeDisposable {
let d1 = vscode.commands.registerCommand('o.restart', () => restartOmniSharp(server));
let d2 = vscode.commands.registerCommand('o.pickProjectAndStart', async () => pickProjectAndStart(server, optionProvider));
let d3 = vscode.commands.registerCommand('o.showOutput', () => eventStream.post(new ShowOmniSharpChannel()));
let d4 = vscode.commands.registerCommand('dotnet.restore', fileName => {
if (fileName) {
dotnetRestoreForProject(server, fileName, eventStream);
}
else {
dotnetRestoreAllProjects(server, eventStream);
}
});
let disposable = new CompositeDisposable();
disposable.add(vscode.commands.registerCommand('o.restart', () => restartOmniSharp(server)));
disposable.add(vscode.commands.registerCommand('o.pickProjectAndStart', async () => pickProjectAndStart(server, optionProvider)));
disposable.add(vscode.commands.registerCommand('o.showOutput', () => eventStream.post(new ShowOmniSharpChannel())));
disposable.add(vscode.commands.registerCommand('dotnet.restore.project', () => pickProjectAndDotnetRestore(server, eventStream)));
disposable.add(vscode.commands.registerCommand('dotnet.restore.all', () => dotnetRestoreAllProjects(server, eventStream)));

// register empty handler for csharp.installDebugger
// running the command activates the extension, which is all we need for installation to kickoff
let d5 = vscode.commands.registerCommand('csharp.downloadDebugger', () => { });
disposable.add(vscode.commands.registerCommand('csharp.downloadDebugger', () => { }));

// register process picker for attach
let attachItemsProvider = DotNetAttachItemsProviderFactory.Get();
let attacher = new AttachPicker(attachItemsProvider);
let d6 = vscode.commands.registerCommand('csharp.listProcess', async () => attacher.ShowAttachEntries());
disposable.add(vscode.commands.registerCommand('csharp.listProcess', async () => attacher.ShowAttachEntries()));

// Register command for generating tasks.json and launch.json assets.
let d7 = vscode.commands.registerCommand('dotnet.generateAssets', async () => generateAssets(server));
disposable.add(vscode.commands.registerCommand('dotnet.generateAssets', async () => generateAssets(server)));

// Register command for remote process picker for attach
let d8 = vscode.commands.registerCommand('csharp.listRemoteProcess', async (args) => RemoteAttachPicker.ShowAttachEntries(args, platformInfo));
disposable.add(vscode.commands.registerCommand('csharp.listRemoteProcess', async (args) => RemoteAttachPicker.ShowAttachEntries(args, platformInfo)));

// Register command for adapter executable command.
let d9 = vscode.commands.registerCommand('csharp.coreclrAdapterExecutableCommand', async (args) => getAdapterExecutionCommand(platformInfo, eventStream));
let d10 = vscode.commands.registerCommand('csharp.clrAdapterExecutableCommand', async (args) => getAdapterExecutionCommand(platformInfo, eventStream));
disposable.add(vscode.commands.registerCommand('csharp.coreclrAdapterExecutableCommand', async (args) => getAdapterExecutionCommand(platformInfo, eventStream)));
disposable.add(vscode.commands.registerCommand('csharp.clrAdapterExecutableCommand', async (args) => getAdapterExecutionCommand(platformInfo, eventStream)));

return new CompositeDisposable(d1, d2, d3, d4, d5, d6, d7, d8, d9, d10);
return new CompositeDisposable(disposable);
}

function restartOmniSharp(server: OmniSharpServer) {
Expand All @@ -64,7 +59,7 @@ function restartOmniSharp(server: OmniSharpServer) {
}
}

async function pickProjectAndStart(server: OmniSharpServer, optionProvider: OptionProvider) {
async function pickProjectAndStart(server: OmniSharpServer, optionProvider: OptionProvider): Promise<void> {
let options = optionProvider.GetLatestOptions();
return findLaunchTargets(options).then(targets => {

Expand Down Expand Up @@ -120,58 +115,40 @@ function projectsToCommands(projects: protocol.ProjectDescriptor[], eventStream:
});
}

export async function dotnetRestoreAllProjects(server: OmniSharpServer, eventStream: EventStream): Promise<void> {

if (!server.isRunning()) {
return Promise.reject('OmniSharp server is not running.');
async function pickProjectAndDotnetRestore(server: OmniSharpServer, eventStream: EventStream): Promise<void> {
let descriptors = await getProjectDescriptors(server);
eventStream.post(new CommandDotNetRestoreStart());
let commands = await Promise.all(projectsToCommands(descriptors, eventStream));
let command = await vscode.window.showQuickPick(commands);
if (command) {
return command.execute();
}

return serverUtils.requestWorkspaceInformation(server).then(async info => {

let descriptors = protocol.getDotNetCoreProjectDescriptors(info);

if (descriptors.length === 0) {
return Promise.reject("No .NET Core projects found");
}

let commandPromises = projectsToCommands(descriptors, eventStream);

return Promise.all(commandPromises).then(commands => {
return vscode.window.showQuickPick(commands);
}).then(command => {
if (command) {
return command.execute();
}
});
});
}

export async function dotnetRestoreForProject(server: OmniSharpServer, filePath: string, eventStream: EventStream) {
async function dotnetRestoreAllProjects(server: OmniSharpServer, eventStream: EventStream): Promise<void> {
let descriptors = await getProjectDescriptors(server);
eventStream.post(new CommandDotNetRestoreStart());
for (let descriptor of descriptors) {
await dotnetRestore(descriptor.Directory, eventStream);
Copy link

Choose a reason for hiding this comment

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

What does the log output look like while we restore multiple projects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shows the logs from the restore of each project
restore

}
}

async function getProjectDescriptors(server: OmniSharpServer): Promise<protocol.ProjectDescriptor[]> {
if (!server.isRunning()) {
return Promise.reject('OmniSharp server is not running.');
}

return serverUtils.requestWorkspaceInformation(server).then(async info => {

let descriptors = protocol.getDotNetCoreProjectDescriptors(info);

if (descriptors.length === 0) {
return Promise.reject("No .NET Core projects found");
}
let info = await serverUtils.requestWorkspaceInformation(server);
let descriptors = protocol.getDotNetCoreProjectDescriptors(info);
if (descriptors.length === 0) {
return Promise.reject("No .NET Core projects found");
}

for (let descriptor of descriptors) {
if (descriptor.FilePath === filePath) {
return dotnetRestore(descriptor.Directory, eventStream, filePath);
}
}
});
return descriptors;
}

async function dotnetRestore(cwd: string, eventStream: EventStream, filePath?: string) {
async function dotnetRestore(cwd: string, eventStream: EventStream, filePath?: string): Promise<void> {
return new Promise<void>((resolve, reject) => {
eventStream.post(new CommandDotNetRestoreStart());

let cmd = 'dotnet';
let args = ['restore'];

Expand Down
5 changes: 3 additions & 2 deletions src/observers/InformationMessageObserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ export class InformationMessageObserver {
}

private async handleOmnisharpServerUnresolvedDependencies(event: ObservableEvent.OmnisharpServerUnresolvedDependencies) {
//to do: determine if we need the unresolved dependencies message
let csharpConfig = this.vscode.workspace.getConfiguration('csharp');
if (!csharpConfig.get<boolean>('suppressDotnetRestoreNotification')) {
let message = `There are unresolved dependencies from '${this.vscode.workspace.asRelativePath(event.unresolvedDependencies.FileName)}'. Please execute the restore command to continue.`;
return showInformationMessage(this.vscode, message, { title: 'Restore', command: 'dotnet.restore', args: event.unresolvedDependencies.FileName });
let message = `There are unresolved dependencies'. Please execute the restore command to continue.`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DustinCampbell Do we want to display the solution name ( the one we display in the status bar item) here -- There are unresolved dependencies from abc.sln ?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe. If we do, the text should probably be in abc.sln rather than from abc.sln.

However, I'm interested in the semantics of how this message appears. If several "unresolved dependencies" events trigger for different projects, will we just display a single message? If so, would it be better if we triggered dotnet restore for each project rather than running it over the whole workspace? Doing the latter might be unexpected.

Copy link

Choose a reason for hiding this comment

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

I think one problem is that discovery of unrestored projects is asynchronous. If we implement the feature so that it only restores projects for which it received the "unrestored project" message, there's a race condition between this popup and the discovery. Eg, one unrestored project is discovered and the warning is shown, and the user clicks restore. O# continues discovering projects and finds more, triggering a new prompt that the user needs to click to restore the newly discovered projects.

Restoring the entire sln file at once avoids that race condition. I suppose restoring the whole solution could be slow, but what else is "unexpected" about it?

There's also the case of multiple csproj's floating around with no sln file. I'm not sure how you would coordinate restoring all of those projects simulatenously without an sln...

return showInformationMessage(this.vscode, message, { title: "Restore", command: "dotnet.restore.all" });
}
}
}
1 change: 0 additions & 1 deletion src/observers/utils/MessageItemWithCommand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@ import { MessageItem } from "../../vscodeAdapter";

export default interface MessageItemWithCommand extends MessageItem {
command: string;
args?: any;
}
9 changes: 2 additions & 7 deletions src/observers/utils/ShowInformationMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,13 @@
*--------------------------------------------------------------------------------------------*/

import { vscode } from "../../vscodeAdapter";
import MessageItemWithCommand from "./MessageItemWithCommand";
import MessageItemWithCommand from "./MessageItemWithCommand";

export default async function showInformationMessage(vscode: vscode, message: string, ...items: MessageItemWithCommand[]) {
try {
let value = await vscode.window.showInformationMessage<MessageItemWithCommand>(message, ...items);
if (value && value.command) {
if (value.args) {
vscode.commands.executeCommand(value.command, value.args);
}
else {
vscode.commands.executeCommand(value.command);
}
vscode.commands.executeCommand(value.command);
}
}
catch (err) {
Expand Down
18 changes: 5 additions & 13 deletions test/unitTests/logging/InformationMessageObserver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ suite("InformationMessageObserver", () => {
let commandDone: Promise<void>;
let vscode = getVsCode();
let infoMessage: string;
let relativePath: string;
let invokedCommand: string;
let observer: InformationMessageObserver = new InformationMessageObserver(vscode);

setup(() => {
infoMessage = undefined;
invokedCommand = undefined;
doClickCancel = undefined;
doClickOk = undefined;
commandDone = new Promise<void>(resolve => {
signalCommandDone = () => { resolve(); };
});
Expand All @@ -35,7 +38,7 @@ suite("InformationMessageObserver", () => {
[
{
event: getUnresolvedDependenices("someFile"),
expectedCommand: "dotnet.restore"
expectedCommand: "dotnet.restore.all"
}
].forEach((elem) => {
suite(elem.event.constructor.name, () => {
Expand All @@ -53,7 +56,6 @@ suite("InformationMessageObserver", () => {

test('The information message is shown', async () => {
observer.post(elem.event);
expect(relativePath).to.not.be.empty;
expect(infoMessage).to.not.be.empty;
doClickOk();
await commandDone;
Expand All @@ -79,11 +81,6 @@ suite("InformationMessageObserver", () => {

teardown(() => {
commandDone = undefined;
infoMessage = undefined;
relativePath = undefined;
invokedCommand = undefined;
doClickCancel = undefined;
doClickOk = undefined;
});

function getVsCode() {
Expand All @@ -107,11 +104,6 @@ suite("InformationMessageObserver", () => {
return undefined;
};

vscode.workspace.asRelativePath = (pathOrUri?: string, includeWorspaceFolder?: boolean) => {
relativePath = pathOrUri;
return relativePath;
};

return vscode;
}
});