From 3a060539c128cfe64dd5fc4cd4d9c1d64baa9d9b Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 23 Jun 2016 15:40:42 -0700 Subject: [PATCH] Add better logging when node is not installed Bug Poor initial experiance when launching NTVS without node installed. Fix Adds a few more logging points: * Log if npm cannot be found * Log if typings tool install fails * Log if typings tool cannot be found. All these are logged to the output window. Closes #1069 --- .../Product/Nodejs/Project/NodeModulesNode.cs | 29 ++++--------------- .../Nodejs/Project/NodejsProjectNode.cs | 15 +++++++++- .../Nodejs/Project/ProjectResources.cs | 13 +++++---- Nodejs/Product/Nodejs/Resources.resx | 13 +++++++-- Nodejs/Product/Nodejs/TypingsAcquisition.cs | 13 +++++---- Nodejs/Product/Npm/Resources.Designer.cs | 17 +++-------- Nodejs/Product/Npm/Resources.resx | 9 ++---- Nodejs/Product/Npm/SPI/NpmCommand.cs | 9 ++---- 8 files changed, 54 insertions(+), 64 deletions(-) diff --git a/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs b/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs index d30721835..7a205d38a 100644 --- a/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs +++ b/Nodejs/Product/Nodejs/Project/NodeModulesNode.cs @@ -193,31 +193,15 @@ private bool HasModules { #region Logging and status bar updates -#if DEV14_OR_LATER - // This is the package manager pane that ships with VS2015, and we should print there if available. - private static readonly Guid VSPackageManagerPaneGuid = new Guid("C7E31C31-1451-4E05-B6BE-D11B6829E8BB"); -#else - private static readonly Guid NpmOutputPaneGuid = new Guid("25764421-33B8-4163-BD02-A94E299D52D8"); -#endif - - private OutputWindowRedirector GetNpmOutputPane() { - try { -#if DEV14_OR_LATER - return OutputWindowRedirector.Get(_projectNode.Site, VSPackageManagerPaneGuid, "Bower/npm"); -#else - return OutputWindowRedirector.Get(_projectNode.Site, NpmOutputPaneGuid, SR.GetString(SR.NpmOutputPaneTitle)); -#endif - } catch (InvalidOperationException) { - return null; + private OutputWindowRedirector NpmOutputPane { + get { + return _projectNode.NpmOutputPane; } } private void ConditionallyShowNpmOutputPane() { if (NodejsPackage.Instance.NpmOptionsPage.ShowOutputWindowWhenExecutingNpm) { - var pane = GetNpmOutputPane(); - if (null != pane) { - pane.ShowAndActivate(); - } + NpmOutputPane?.ShowAndActivate(); } } @@ -300,10 +284,7 @@ private static string TrimLastNewline(string text) { } private void WriteNpmLogToOutputWindow(string logText) { - var pane = GetNpmOutputPane(); - if (null != pane) { - pane.WriteLine(logText); - } + NpmOutputPane?.WriteLine(logText); #if INTEGRATE_WITH_ERROR_LIST WriteNpmErrorsToErrorList(args); diff --git a/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs b/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs index 0c0541482..113187e1a 100644 --- a/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs +++ b/Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs @@ -159,7 +159,7 @@ private void TryToAcquireTypings(IEnumerable packages) { var typingsPath = Path.Combine(this.ProjectHome, "typings"); bool hadExistingTypingsFolder = Directory.Exists(typingsPath); TypingsAcquirer - .AcquireTypings(packages, null /*redirector*/) + .AcquireTypings(packages, NpmOutputPane) .ContinueWith(x => { if (NodejsPackage.Instance.IntellisenseOptionsPage.ShowTypingsInfoBar && x.Result && @@ -1246,5 +1246,18 @@ public override MSBuildResult Build(string config, string target) { return base.Build(config, target); } + + // This is the package manager pane that ships with VS2015, and we should print there if available. + private static readonly Guid VSPackageManagerPaneGuid = new Guid("C7E31C31-1451-4E05-B6BE-D11B6829E8BB"); + + internal OutputWindowRedirector NpmOutputPane { + get { + try { + return OutputWindowRedirector.Get(Site, VSPackageManagerPaneGuid, "Bower/npm"); + } catch (InvalidOperationException) { + return null; + } + } + } } } diff --git a/Nodejs/Product/Nodejs/Project/ProjectResources.cs b/Nodejs/Product/Nodejs/Project/ProjectResources.cs index b80065269..040c2a4ef 100644 --- a/Nodejs/Product/Nodejs/Project/ProjectResources.cs +++ b/Nodejs/Product/Nodejs/Project/ProjectResources.cs @@ -73,6 +73,7 @@ internal class SR : CommonSR { internal const string NewVersionPackageCatalogNotRetrieved = "NewVersionPackageCatalogNotRetrieved"; internal const string NewVersionUnknown = "NewVersionUnknown"; internal const string NewVersionYes = "NewVersionYes"; + internal const string NoKeywordsInPackage = "NoKeywordsInPackage"; internal const string NodeExeArguments = "NodeExeArguments"; internal const string NodeExeArgumentsDescription = "NodeExeArgumentsDescription"; internal const string NodeExeArgumentsToolTip = "NodeExeArgumentsToolTip"; @@ -87,7 +88,6 @@ internal class SR : CommonSR { internal const string NodejsPort = "NodejsPort"; internal const string NodejsPortDescription = "NodejsPortDescription"; internal const string NodejsPortToolTip = "NodejsPortToolTip"; - internal const string NoKeywordsInPackage = "NoKeywordsInPackage"; internal const string NpmCancelled = "NpmCancelled"; internal const string NpmCancelledWithErrors = "NpmCancelledWithErrors"; internal const string NpmCompletedWithErrors = "NpmCompletedWithErrors"; @@ -173,8 +173,8 @@ internal class SR : CommonSR { internal const string ScriptFileToolTip = "ScriptFileTooltip"; internal const string Seconds = "Seconds"; internal const string StartBrowserToolTip = "StartBrowserToolTip"; - internal const string StatusAnalysisLoaded = "StatusAnalysisLoaded"; internal const string StatusAnalysisLoadFailed = "StatusAnalysisLoadFailed"; + internal const string StatusAnalysisLoaded = "StatusAnalysisLoaded"; internal const string StatusAnalysisLoading = "StatusAnalysisLoading"; internal const string StatusAnalysisSaved = "StatusAnalysisSaved"; internal const string StatusAnalysisSaving = "StatusAnalysisSaving"; @@ -187,14 +187,15 @@ internal class SR : CommonSR { internal const string TypingsInfoBarSpan1 = "TypingsInfoBarSpan1"; internal const string TypingsInfoBarSpan2 = "TypingsInfoBarSpan2"; internal const string TypingsInfoBarSpan3 = "TypingsInfoBarSpan3"; - internal const string TypingsToolInstallCompleted = "TypingsToolInstallCompleted"; - internal const string TypingsToolInstallErrorOccurred = "TypingsToolInstallErrorOccurred"; - internal const string TypingsToolNotInstalledError = "TypingsToolNotInstalledError"; internal const string TypingsOpenOptionsText = "TypingsOpenOptionsText"; + internal const string TypingsToolCouldNotStart = "TypingsToolCouldNotStart"; + internal const string TypingsToolInstallFailed = "TypingsToolInstallFailed"; + internal const string TypingsToolNotInstalledError = "TypingsToolNotInstalledError"; + internal const string TypingsToolTypingsInstallCompleted = "TypingsToolTypingsInstallCompleted"; + internal const string TypingsToolTypingsInstallErrorOccurred = "TypingsToolTypingsInstallErrorOccurred"; internal const string UpgradedEnvironmentVariables = "UpgradedEnvironmentVariables"; internal const string WorkingDirInvalidOrMissing = "WorkingDirInvalidOrMissing"; internal const string WorkingDirToolTip = "WorkingDirToolTip"; - private static readonly Lazy _manager = new Lazy( () => new System.Resources.ResourceManager("Microsoft.NodejsTools.Resources", typeof(SR).Assembly), LazyThreadSafetyMode.ExecutionAndPublication diff --git a/Nodejs/Product/Nodejs/Resources.resx b/Nodejs/Product/Nodejs/Resources.resx index b72bcd3ec..330d2736a 100644 --- a/Nodejs/Product/Nodejs/Resources.resx +++ b/Nodejs/Product/Nodejs/Resources.resx @@ -609,11 +609,14 @@ You will need to restart Visual Studio after installation. Could not find RemoteDebugProxyFolder - + Successfully installed typing files for Intellisense. - Could not find Typings package manager tool used for Intellisense. + Could not find Typings package manager tool used for Intellisense + + + An error occurred installing typings used for IntelliSense. Node.js IntelliSense added a @@ -639,4 +642,10 @@ You will need to restart Visual Studio after installation. Loading Node.js IntelliSense typings for npm packages... + + Failed to install Typings tool used for IntelliSense. Please make sure Node.js is properly installed + + + Could not start Typings tool used for IntelliSense + \ No newline at end of file diff --git a/Nodejs/Product/Nodejs/TypingsAcquisition.cs b/Nodejs/Product/Nodejs/TypingsAcquisition.cs index 196566496..2154b370a 100644 --- a/Nodejs/Product/Nodejs/TypingsAcquisition.cs +++ b/Nodejs/Product/Nodejs/TypingsAcquisition.cs @@ -112,7 +112,7 @@ private IEnumerable GetNewTypingsToAcquire(IEnumerable packages) } private async Task ExecuteTypingsTool(IEnumerable arguments, Redirector redirector) { - string typingsTool = await EnsureTypingsToolInstalled(); + string typingsTool = await EnsureTypingsToolInstalled(redirector); if (string.IsNullOrEmpty(typingsTool)) { redirector?.WriteErrorLine(SR.GetString(SR.TypingsToolNotInstalledError)); return false; @@ -129,22 +129,22 @@ private async Task ExecuteTypingsTool(IEnumerable arguments, Redir if (!process.IsStarted) { // Process failed to start, and any exception message has // already been sent through the redirector - redirector?.WriteErrorLine("could not start 'typings'"); + redirector?.WriteErrorLine(SR.GetString(SR.TypingsToolCouldNotStart)); return false; } var i = await process; if (i == 0) { - redirector?.WriteLine(SR.GetString(SR.TypingsToolInstallCompleted)); + redirector?.WriteLine(SR.GetString(SR.TypingsToolTypingsInstallCompleted)); return true; } else { process.Kill(); - redirector?.WriteErrorLine(SR.GetString(SR.TypingsToolInstallErrorOccurred)); + redirector?.WriteErrorLine(SR.GetString(SR.TypingsToolTypingsInstallErrorOccurred)); return false; } } } - private async Task EnsureTypingsToolInstalled() { + private async Task EnsureTypingsToolInstalled(Redirector redirector) { if (File.Exists(TypingsToolPath)) { return TypingsToolPath; } @@ -153,9 +153,10 @@ private async Task EnsureTypingsToolInstalled() { return null; } if (!await InstallTypingsTool()) { + redirector?.WriteErrorLine(SR.GetString(SR.TypingsToolInstallFailed)); return null; } - return await EnsureTypingsToolInstalled(); + return await EnsureTypingsToolInstalled(redirector); } private async Task InstallTypingsTool() { diff --git a/Nodejs/Product/Npm/Resources.Designer.cs b/Nodejs/Product/Npm/Resources.Designer.cs index 15c163ed8..86eca6c98 100644 --- a/Nodejs/Product/Npm/Resources.Designer.cs +++ b/Nodejs/Product/Npm/Resources.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.34014 +// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -61,20 +61,11 @@ internal Resources() { } /// - /// Looks up a localized string similar to Cannot retrieve license from empty license collection.. + /// Looks up a localized string similar to Could not find npm. Please make sure Node.js is installed. /// - internal static string CannotRetrieveLicenseEmptyCollection { + internal static string CouldNotFindNpm { get { - return ResourceManager.GetString("CannotRetrieveLicenseEmptyCollection", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Cannot retrieve license for index less than 0.. - /// - internal static string CannotRetrieveLicenseInvalidIndex { - get { - return ResourceManager.GetString("CannotRetrieveLicenseInvalidIndex", resourceCulture); + return ResourceManager.GetString("CouldNotFindNpm", resourceCulture); } } diff --git a/Nodejs/Product/Npm/Resources.resx b/Nodejs/Product/Npm/Resources.resx index 83e2346f2..a9e6a4373 100644 --- a/Nodejs/Product/Npm/Resources.resx +++ b/Nodejs/Product/Npm/Resources.resx @@ -132,12 +132,6 @@ Downloaded {0}MB of the package list ({1}MB total) - - Cannot retrieve license from empty license collection. - - - Cannot retrieve license for index less than 0. - Error executing npm - unable to start the npm process @@ -204,4 +198,7 @@ Semversion {0} for package {1} is invalid args: semantic version, package name + + Could not find npm. Please make sure Node.js is installed + \ No newline at end of file diff --git a/Nodejs/Product/Npm/SPI/NpmCommand.cs b/Nodejs/Product/Npm/SPI/NpmCommand.cs index d5e40a870..6525ef846 100644 --- a/Nodejs/Product/Npm/SPI/NpmCommand.cs +++ b/Nodejs/Product/Npm/SPI/NpmCommand.cs @@ -15,16 +15,11 @@ //*********************************************************// using System; -using System.Collections; -using System.Collections.Generic; -using System.ComponentModel; -using System.Diagnostics; using System.IO; using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.VisualStudioTools.Project; -using Microsoft.Win32; namespace Microsoft.NodejsTools.Npm.SPI { internal abstract class NpmCommand : AbstractNpmLogSource { @@ -78,12 +73,14 @@ public void CancelCurrentTask() { public virtual async Task ExecuteAsync() { OnCommandStarted(); + var redirector = new NpmCommandRedirector(this); + try { GetPathToNpm(); } catch (NpmNotFoundException) { + redirector.WriteErrorLine(Resources.CouldNotFindNpm); return false; } - var redirector = new NpmCommandRedirector(this); redirector.WriteLine( string.Format("===={0}====\r\n\r\n", string.Format(Resources.ExecutingCommand, Arguments)));