Skip to content

Change the command to raise when adding new project from existing code#396

Merged
mousetraps merged 9 commits intomicrosoft:masterfrom
zhengbli:issue133
Sep 9, 2015
Merged

Change the command to raise when adding new project from existing code#396
mousetraps merged 9 commits intomicrosoft:masterfrom
zhengbli:issue133

Conversation

@zhengbli
Copy link
Copy Markdown

This PR fixes #133.

Previously the raised command when adding new project from existing code is VSConstants.VSStd97CmdID.OpenProject, which actually corresponds to the Open Project menu item, that will always create a new solution. Changing it to VSConstants.VSStd97CmdID.AddExistingProject fixed the problem.

@paulvanbrenk
Copy link
Copy Markdown
Contributor

👍

@billti
Copy link
Copy Markdown
Member

billti commented Aug 28, 2015

Thanks for figuring this out!

Now you are calling AddProject instead of OpenProject, I can see how this fixes the issue referenced. Did you also check that running the "Import from existing code" scenario with no existing project/sln open still continues to work also?

@zhengbli
Copy link
Copy Markdown
Author

Right, after some tests it didn't work for the no-existing project/sln scenario. I need to fix that.

@zhengbli
Copy link
Copy Markdown
Author

@billti Added code to detect the availability of the "Add Existing Project", now it works as expected in both cases.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this work in the instance where a project is open (so the AddExistingProject command is available), but someone is nonetheless creating a new project?

Perhaps a better solution is to intercept the command in the NodejsPackage.Initialize() step by handling BeforeExecute and AfterExecute events (set a boolean flag that we check in ImportWizardCommand).
E.g. http://stackoverflow.com/questions/15406523/visual-studio-extensibility-intercepting-file-paste-event

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternatively, just pass in the cmdIdToRaise rather than effectively checking it twice.

Then, you can can simplify all this to:

NodejsPackage.Instance.DTE.Commands.Raise(
VSConstants.GUID_VSStandardCommandSet97.ToString("B"), cmdIdToRaise, ref pathRef, ref outRef);

@mousetraps
Copy link
Copy Markdown
Contributor

Overall implementation looks good now - left several minor comments and lgtm after that.

@zhengbli
Copy link
Copy Markdown
Author

zhengbli commented Sep 8, 2015

@mousetraps can you take a look at my updates? Thanks!

@mousetraps
Copy link
Copy Markdown
Contributor

👍

mousetraps added a commit that referenced this pull request Sep 9, 2015
Fix #133 - Change the command to raise when adding new project from existing code
@mousetraps mousetraps merged commit f6f0ed8 into microsoft:master Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding new Node.js project from existing code removes all other projects from solution

5 participants