Skip to content

Allow running .npm commands in Repl Window from any proj types. #808

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

Merged
mjbvz merged 9 commits into from
May 19, 2016
Merged

Allow running .npm commands in Repl Window from any proj types. #808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
81ab8ba
Allows NTVS repl window to work for any project type
mjbvz Mar 31, 2016
3105515
Support mixed node + other projects better.
mjbvz Mar 31, 2016
bac5f1f
Remove npm init logic
mjbvz Mar 31, 2016
602aa9c
Remove prefer node project logic
mjbvz Mar 31, 2016
b19622a
Replace IgnoreCase
mjbvz Mar 31, 2016
db921f9
Merge branch 'master' into npm-interactive-from-any-proj
mjbvz Apr 5, 2016
1d41401
Address small cr feedbacks
mjbvz Apr 5, 2016
dced2e7
Prevent running `.npm init` without the yes flag directly in the Repl…
mjbvz Apr 5, 2016
5df5d69
Restore original ProjectHomeLogic, plus fallback for other projecs types
mjbvz Apr 5, 2016
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
1 change: 1 addition & 0 deletions Nodejs/Product/Nodejs/Project/ProjectResources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ internal class SR : CommonSR {
internal const string PropertiesClassLocalSubPackage = "PropertiesClassLocalSubPackage";
internal const string PropertiesClassNpm = "PropertiesClassNpm";
internal const string ReplInitializationMessage = "ReplInitializationMessage";
internal const string ReplWindowNpmInitNoYesFlagWarning = "ReplWindowNpmInitNoYesFlagWarning";
internal const string RequestedVersionRangeNone = "RequestedVersionRangeNone";
internal const string ScriptArgumentsToolTip = "ScriptArgumentsToolTip";
internal const string ScriptFileToolTip = "ScriptFileTooltip";
Expand Down
42 changes: 31 additions & 11 deletions Nodejs/Product/Nodejs/Repl/NpmReplCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,15 @@ public async Task<ExecutionResult> Execute(IReplWindow window, string arguments)
// at beginning and end of string (e.g. '--global')
npmArguments = string.Format(" {0} ", npmArguments);

// Prevent running `npm init` without the `-y` flag since it will freeze the repl window,
// waiting for user input that will never come.
if (npmArguments.Contains(" init ") && !(npmArguments.Contains(" -y ") || npmArguments.Contains(" --yes "))) {
window.WriteError(SR.GetString(SR.ReplWindowNpmInitNoYesFlagWarning));
return ExecutionResult.Failure;
}

var solution = Package.GetGlobalService(typeof(SVsSolution)) as IVsSolution;
IEnumerable<IVsProject> loadedProjects = solution.EnumerateLoadedProjects(onlyNodeProjects: true);
IEnumerable<IVsProject> loadedProjects = solution.EnumerateLoadedProjects(onlyNodeProjects: false);

var projectNameToDirectoryDictionary = new Dictionary<string, Tuple<string, IVsHierarchy>>(StringComparer.OrdinalIgnoreCase);
foreach (IVsProject project in loadedProjects) {
Expand All @@ -76,19 +83,33 @@ public async Task<ExecutionResult> Execute(IReplWindow window, string arguments)
continue;
}

EnvDTE.Properties properties = dteProject.Properties;
if (dteProject.Properties == null) {
string projectName = dteProject.Name;
if (string.IsNullOrEmpty(projectName)) {
continue;
Copy link
Copy Markdown
Contributor

@mousetraps mousetraps Apr 5, 2016

Choose a reason for hiding this comment

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

add back null check for projectName

}

string projectName = dteProject.Name;
EnvDTE.Property projectHome = properties.Item("ProjectHome");
if (projectHome == null || projectName == null) {
continue;
// Try checking the `ProjectHome` property first
EnvDTE.Properties properties = dteProject.Properties;
if (dteProject.Properties != null) {
EnvDTE.Property projectHome = null;
try {
projectHome = properties.Item("ProjectHome");
} catch (ArgumentException) {
// noop
}

if (projectHome != null) {
var projectHomeDirectory = projectHome.Value as string;
if (!string.IsNullOrEmpty(projectHomeDirectory)) {
projectNameToDirectoryDictionary.Add(projectName, Tuple.Create(projectHomeDirectory, hierarchy));
continue;
}
}
}

var projectDirectory = projectHome.Value as string;
if (projectDirectory != null) {
// Otherwise, fall back to using fullname
string projectDirectory = Path.GetDirectoryName(dteProject.FullName);
Copy link
Copy Markdown
Contributor

@mousetraps mousetraps Apr 5, 2016

Choose a reason for hiding this comment

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

this won't work when the project file is in a different location than the directory itself.

Copy link
Copy Markdown
Contributor Author

@mjbvz mjbvz Apr 5, 2016

Choose a reason for hiding this comment

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

Surely no one would do such an unthinkable thing ;)

Any thoughts on what property to use on the dteProject or the projectHome to get this then? Do you recommend restoring the previous ProjectHome logic for use on node projects, and only using this fallback logic for other project types?

Copy link
Copy Markdown
Contributor Author

@mjbvz mjbvz Apr 5, 2016

Choose a reason for hiding this comment

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

Here's an example of what EnvDte.properties contains for a asp app (only printing out string property values):

Count = 102
    [0]: {(WebApplication.AspNetDebugging, )}
    [1]: {(WebApplication.AspnetCompilerIISMetabasePath, )}
    [2]: {(OutputTypeEx, )}
    [3]: {(TargetFrameworkMoniker, .NETFramework,Version=v4.5.2)}
    [4]: {(ComVisible, )}
    [5]: {(EnableSecurityDebugging, )}
    [6]: {(OptionCompare, )}
    [7]: {(StartupObject, )}
    [8]: {(WebApplication.SSLEnabled, )}
    [9]: {(WebApplication.UseIIS, )}
    [10]: {(ManifestCertificateThumbprint, )}
    [11]: {(Trademark, )}
    [12]: {(Title, WebApplication1)}
    [13]: {(WebApplication.StartExternalUrl, )}
    [14]: {(WebApplication.IISUrl, http://localhost:63763/)}
    [15]: {(AssemblyOriginatorKeyFileType, )}
    [16]: {(FileName, WebApplication1.csproj)}
    [17]: {(WebServer, )}
    [18]: {(AssemblyOriginatorKeyMode, )}
    [19]: {(AssemblyKeyContainerName, )}
    [20]: {(WebApplication.WindowsAuthenticationEnabled, )}
    [21]: {(WebApplication.SecureUrl, )}
    [22]: {(WebApplication.DevelopmentServerCommandLine,  /config:"C:\Users\matb.REDMOND\Documents\Visual Studio 2015\Projects\WebApplication1\.vs\config\applicationhost.config" /site:"WebApplication1" /apppool:"Clr4IntegratedAppPool")}
    [23]: {(WebApplication.SQLDebugging, )}
    [24]: {(WebApplication.StartPageUrl, )}
    [25]: {(WebApplication.DefaultServerDirectoryListing, )}
    [26]: {(WebApplication.DevelopmentServerVPath, /)}
    [27]: {(WebApplication.DevelopmentServerPort, )}
    [28]: {(ProjectType, )}
    [29]: {(ReferencePath, )}
    [30]: {(WebApplication.IsUsingIISExpress, )}
    [31]: {(PreBuildEvent, )}
    [32]: {(WebApplication.AnonymousAuthenticationEnabled, )}
    [33]: {(WebApplication.SilverlightDebugging, )}
    [34]: {(WebApplication.StartCmdLineArguments, )}
    [35]: {(Copyright, Copyright ©  2016)}
    [36]: {(ApplicationIcon, )}
    [37]: {(WebApplication.CurrentDebugUrl, http://localhost:63763/)}
    [38]: {(ExcludedPermissions, )}
    [39]: {(RunPostBuildEvent, )}
    [40]: {(DefaultTargetSchema, )}
    [41]: {(RootNamespace, WebApplication1)}
    [42]: {(WebApplication.IsUsingCustomServer, )}
    [43]: {(ManifestTimestampUrl, )}
    [44]: {(ManifestKeyFile, )}
    [45]: {(DebugSecurityZoneURL, )}
    [46]: {(Product, WebApplication1)}
    [47]: {(PostBuildEvent, )}
    [48]: {(OptionStrict, )}
    [49]: {(DefaultHTMLPageLayout, )}
    [50]: {(DelaySign, )}
    [51]: {(OutputType, )}
    [52]: {(WebApplication.StartWorkingDirectory, )}
    [53]: {(WebApplication.DebugStartAction, )}
    [54]: {(NeutralResourcesLanguage, )}
    [55]: {(OptionExplicit, )}
    [56]: {(OutputFileName, WebApplication1.dll)}
    [57]: {(ServerExtensionsVersion, )}
    [58]: {(WebApplication.NonSecureUrl, http://localhost:63763/)}
    [59]: {(WebApplication.ToString, Microsoft.VisualStudio.Web.Application.WAProjectExtender)}
    [60]: {(AssemblyGuid, 295f02da-71a5-4ad9-a3b4-cfc9544dd82a)}
    [61]: {(GenerateManifests, )}
    [62]: {(AssemblyVersion, 1.0.0.0)}
    [63]: {(Win32ResourceFile, )}
    [64]: {(Description, )}
    [65]: {(URL, file:///C:\Users\matb.REDMOND\Documents\Visual Studio 2015\Projects\WebApplication1\WebApplication1\)}
    [66]: {(DefaultClientScript, )}
    [67]: {(WebApplication.NativeDebugging, )}
    [68]: {(TargetFramework, )}
    [69]: {(SignManifests, )}
    [70]: {(OfflineURL, )}
    [71]: {(WebServerVersion, )}
    [72]: {(Publish, )}
    [73]: {(AssemblyType, )}
    [74]: {(FullPath, C:\Users\matb.REDMOND\Documents\Visual Studio 2015\Projects\WebApplication1\WebApplication1\)}
    [75]: {(WebAccessMethod, )}
    [76]: {(WebApplication.UseIISExpress, )}
    [77]: {(WebApplication.BrowseURL, http://localhost:63763)}
    [78]: {(WebApplication.NTLMAuthentication, )}
    [79]: {(WebApplication.OverrideIISAppRootUrl, )}
    [80]: {(WebApplication.OpenedURL, file:///C:/Users/matb.REDMOND/Documents/Visual Studio 2015/Projects/WebApplication1/WebApplication1/)}
    [81]: {(AssemblyKeyProviderName, )}
    [82]: {(TypeComplianceDiagnostics, )}
    [83]: {(Company, )}
    [84]: {(ActiveFileSharePath, )}
    [85]: {(AssemblyOriginatorKeyFile, )}
    [86]: {(WebApplication.StartWebServerOnDebug, )}
    [87]: {(WebApplication.AutoAssignPort, )}
    [88]: {(ApplicationManifest, DefaultManifest)}
    [89]: {(AssemblyFileVersion, 1.0.0.0)}
    [90]: {(AspnetVersion, )}
    [91]: {(FileSharePath, )}
    [92]: {(AssemblyName, WebApplication1)}
    [93]: {(WebApplication.EditAndContinue, )}
    [94]: {(WebApplication.ServerDirectoryListing, )}
    [95]: {(LocalPath, C:\Users\matb.REDMOND\Documents\Visual Studio 2015\Projects\WebApplication1\WebApplication1\)}
    [96]: {(DefaultNamespace, WebApplication1)}
    [97]: {(LinkRepair, )}
    [98]: {(WebApplication.StartExternalProgram, )}
    [99]: {(WebApplication.IISAppRootUrl, )}
    [0]: {(TargetZone, )}
    [1]: {(SignAssembly, )}

if (!string.IsNullOrEmpty(projectDirectory)) {
projectNameToDirectoryDictionary.Add(projectName, Tuple.Create(projectDirectory, hierarchy));
}
}
Copy link
Copy Markdown
Contributor Author

@mjbvz mjbvz Mar 31, 2016

Choose a reason for hiding this comment

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

This part may be attempting to be too clever.

In a multi-project solution, it may be better to require users to explicitly state which project they want to run NPM for. Currently, if there is any Node project in a solution, you can't run .npm for any of the other projects in that solution.

Expand Down Expand Up @@ -118,7 +139,7 @@ public async Task<ExecutionResult> Execute(IReplWindow window, string arguments)
// In case someone copies filename
string projectDirectoryPath = File.Exists(projectPath) ? Path.GetDirectoryName(projectPath) : projectPath;

if (!isGlobalCommand && !(Directory.Exists(projectDirectoryPath) && File.Exists(Path.Combine(projectDirectoryPath, "package.json")))) {
if (!isGlobalCommand && !Directory.Exists(projectDirectoryPath)) {
Copy link
Copy Markdown
Contributor

@mousetraps mousetraps Apr 5, 2016

Choose a reason for hiding this comment

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

you'll want to block npm init here, otherwise the repl will freeze up.

Copy link
Copy Markdown
Contributor

@mousetraps mousetraps Apr 5, 2016

Choose a reason for hiding this comment

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

in the event of .npm init, we should warn people and ask them to run .npm init -y instead which will accept all the defaults rather than waiting for stdio

Copy link
Copy Markdown
Contributor Author

@mjbvz mjbvz Apr 5, 2016

Choose a reason for hiding this comment

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

Adding a check for init sounds fine for now.

This problem is also related to #578, where someone runs .npm start with non terminating apps.

window.WriteError("Please specify a valid Node.js project or project directory. If your solution contains multiple projects, specify a target project using .npm [ProjectName or ProjectDir] <npm arguments> For example: .npm [MyApp] list");
return ExecutionResult.Failure;
}
Expand All @@ -137,7 +158,6 @@ public async Task<ExecutionResult> Execute(IReplWindow window, string arguments)
}

var npmReplRedirector = new NpmReplRedirector(window);

await ExecuteNpmCommandAsync(
npmReplRedirector,
npmPath,
Expand Down
3 changes: 3 additions & 0 deletions Nodejs/Product/Nodejs/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ NAME2=value2
<data name="ReplInitializationMessage" xml:space="preserve">
<value>Node.js interactive window. Type .help for a list of commands.</value>
</data>
<data name="ReplWindowNpmInitNoYesFlagWarning" xml:space="preserve">
<value>Please run '.npm init -y' to create a new package.json file.</value>
</data>
<data name="NodeExePathDescription" xml:space="preserve">
<value>Specifies the path to the node.exe executable.</value>
</data>
Expand Down