Merge pull request #1913 from paulvanbrenk/anyCodeFixes

Any code fixes

Author: paulvanbrenk

Common/Product/SharedProject/ProcessOutput.cs

@@ -162,13 +162,19 @@ public static ProcessOutput Run(
                     QuoteSingleArgument(filename),
                     GetArguments(arguments, quoteArgs)),
                 CreateNoWindow = !visible,
-                UseShellExecute = false,
-                RedirectStandardError = !visible || (redirector != null),
-                RedirectStandardOutput = !visible || (redirector != null),
-                RedirectStandardInput = !visible
+                UseShellExecute = false
             };
-            psi.StandardOutputEncoding = outputEncoding ?? psi.StandardOutputEncoding;
-            psi.StandardErrorEncoding = errorEncoding ?? outputEncoding ?? psi.StandardErrorEncoding;
+
+            if (!visible || (redirector != null))
+            {
+                psi.RedirectStandardError = true;
+                psi.RedirectStandardOutput = true;
+                psi.RedirectStandardInput = true;
+                // only set the encoding when we're redirecting the output
+                psi.StandardOutputEncoding = outputEncoding ?? psi.StandardOutputEncoding;
+                psi.StandardErrorEncoding = errorEncoding ?? outputEncoding ?? psi.StandardErrorEncoding;
+            }
+
             if (env != null)
             {
                 foreach (var kv in env)

Nodejs/Product/Nodejs/SharedProject/OutputWindowWrapper.cs

@@ -17,95 +17,57 @@ public sealed class OutputPaneWrapper
 
         // 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");
-
         private static readonly Guid TscPaneGuid = new Guid("7CCFA622-001E-4459-9D8B-5B60BE0A18C2");
 
         [Import]
         private SVsServiceProvider ServiceProvider { get; set; }
 
         private Dictionary<OutputWindowTarget, IVsOutputWindowPane> lazyOutputPaneCollection = new Dictionary<OutputWindowTarget, IVsOutputWindowPane>();
-        private IVsWindowFrame lazyOutputWindow;
 
-        private IVsOutputWindowPane GetOutputPane(OutputWindowTarget target)
+        private IVsOutputWindowPane InitializeOutputPane(string title, Guid paneId)
         {
-            if (this.lazyOutputPaneCollection.TryGetValue(target, out var lazyOutputPane))
-            {
-                return lazyOutputPane;
-            }
-            else
-            {
-                ThreadHelper.ThrowIfNotOnUIThread();
-                var (title, guid) = GetPaneInfo(target);
-                var outputWindow = (IVsOutputWindow)this.ServiceProvider.GetService(typeof(SVsOutputWindow));
-
-                // Try to get the workspace pane if it has already been registered
-                var hr = outputWindow.GetPane(guid, out lazyOutputPane);
-
-                // If the workspace pane has not been registered before, create it
-                if (lazyOutputPane == null || ErrorHandler.Failed(hr))
-                {
-
-                    if (ErrorHandler.Failed(outputWindow.CreatePane(guid, title, fInitVisible: 1, fClearWithSolution: 1)) ||
-                        ErrorHandler.Failed(outputWindow.GetPane(guid, out lazyOutputPane)))
-                    {
-                        return null;
-                    }
-
-                    // Must activate the workspace pane for it to show up in the output window
-                    lazyOutputPane.Activate();
-                }
+            ThreadHelper.ThrowIfNotOnUIThread();
+            var outputWindow = (IVsOutputWindow)this.ServiceProvider.GetService(typeof(SVsOutputWindow));
 
-                this.lazyOutputPaneCollection.Add(target, lazyOutputPane);
-                return lazyOutputPane;
-            }
+            // Try to get the workspace pane if it has already been registered
+            var hr = outputWindow.GetPane(paneId, out var lazyOutputPane);
 
-            (string title, Guid guid) GetPaneInfo(OutputWindowTarget pane)
+            // If the workspace pane has not been registered before, create it
+            if (lazyOutputPane == null || ErrorHandler.Failed(hr))
             {
-                switch (pane)
-                {
-                    case OutputWindowTarget.Npm:
-                        return ("Npm", VSPackageManagerPaneGuid);
-                    case OutputWindowTarget.Tsc:
-                        return ("Tsc", TscPaneGuid); /* no need to localize since name of exe */
-                    default:
-                        throw new ArgumentOutOfRangeException(nameof(pane));
-                }
-            }
-        }
 
-        private IVsWindowFrame OutputWindow
-        {
-            get
-            {
-                if (this.lazyOutputWindow == null && this.ServiceProvider.GetService(typeof(SVsUIShell)) is IVsUIShell shell)
+                if (ErrorHandler.Failed(outputWindow.CreatePane(paneId, title, fInitVisible: 1, fClearWithSolution: 1)) ||
+                    ErrorHandler.Failed(outputWindow.GetPane(paneId, out lazyOutputPane)))
                 {
-                    ThreadHelper.ThrowIfNotOnUIThread();
-
-                    var windowGuid = OutputWindowGuid;
-                    var hr = shell.FindToolWindow((int)__VSFINDTOOLWIN.FTW_fForceCreate, ref windowGuid, out this.lazyOutputWindow);
-                    Debug.Assert(ErrorHandler.Succeeded(hr));
+                    return null;
                 }
 
-                return this.lazyOutputWindow;
+                // Must activate the workspace pane for it to show up in the output window
+                lazyOutputPane.Activate();
             }
+
+            return lazyOutputPane;
         }
 
         public void WriteLine(string message, OutputWindowTarget target = OutputWindowTarget.Npm)
         {
-            if (this.lazyOutputWindow == null)
+            if (!this.lazyOutputPaneCollection.TryGetValue(target, out var lazyOutputPane) || lazyOutputPane == null)
             {
-                throw new InvalidOperationException($"Ensure the output window is initialized by calling '{nameof(ShowWindow)}' first.");
+                throw new InvalidOperationException("You need to initialize the output panes before using them.");
             }
 
-            var hr = this.GetOutputPane(target).OutputStringThreadSafe(message + Environment.NewLine);
+            var hr = lazyOutputPane.OutputStringThreadSafe(message + Environment.NewLine);
             Debug.Assert(ErrorHandler.Succeeded(hr));
         }
 
-        public void ShowWindow()
+        public void InitializeOutputPanes()
         {
             ThreadHelper.ThrowIfNotOnUIThread();
-            var hr = this.OutputWindow?.ShowNoActivate() ?? VSConstants.E_FAIL;
-            Debug.Assert(ErrorHandler.Succeeded(hr));
+
+            var npmPane = InitializeOutputPane("Npm", VSPackageManagerPaneGuid);
+            this.lazyOutputPaneCollection.Add(OutputWindowTarget.Npm, npmPane);
+            var tscPane = InitializeOutputPane("Tsc", TscPaneGuid);
+            this.lazyOutputPaneCollection.Add(OutputWindowTarget.Tsc, tscPane);
         }
     }
 

Nodejs/Product/Nodejs/Workspace/ContextMenuProvider.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using System.Diagnostics;
+using System.IO;
 using System.Linq;
 using System.Threading;
 using Microsoft.NodejsTools.Npm;
@@ -75,26 +76,28 @@ public int Exec(List<WorkspaceVisualNodeBase> selection, Guid pguidCmdGroup, uin
 
                 if (pguidCmdGroup == Guids.NodeToolsWorkspaceCmdSet)
                 {
-                    this.outputPane.ShowWindow();
+                    this.outputPane.InitializeOutputPanes();
+
+                    var fileNode = ((IFileNode)node).FullPath;
 
                     switch (nCmdID)
                     {
                         case PkgCmdId.cmdidWorkSpaceNpmInstallMissing:
-                            ExecNpmInstallMissing(node);
+                            ExecNpmInstallMissing(fileNode);
                             return VSConstants.S_OK;
 
                         case PkgCmdId.cmdidWorkSpaceNpmInstallNew:
-                            ExecNpmInstallNew(node);
+                            ExecNpmInstallNew(fileNode);
                             return VSConstants.S_OK;
 
                         case PkgCmdId.cmdidWorkSpaceNpmUpdate:
-                            ExecNpmUpdate(node);
+                            ExecNpmUpdate(fileNode);
                             return VSConstants.S_OK;
                     }
 
                     if (nCmdID >= PkgCmdId.cmdidWorkSpaceNpmDynamicScript && nCmdID < PkgCmdId.cmdidWorkSpaceNpmDynamicScriptMax)
                     {
-                        ExecDynamic(node, nCmdID);
+                        ExecDynamic(fileNode, nCmdID);
                         return VSConstants.S_OK;
                     }
                 }
@@ -116,44 +119,42 @@ public int Exec(List<WorkspaceVisualNodeBase> selection, Guid pguidCmdGroup, uin
             // Note: all the Exec commands are async, this allows us to call them in a fire and forget
             // pattern, without blocking the UI or losing any logging
 
-            private async void ExecNpmInstallMissing(WorkspaceVisualNodeBase node)
+            private async void ExecNpmInstallMissing(string filePath)
             {
-                using (var npmController = this.CreateController(node.Workspace))
+                using (var npmController = this.CreateController(filePath))
                 using (var commander = npmController.CreateNpmCommander())
                 {
                     await commander.Install();
                 }
             }
 
-            private void ExecNpmInstallNew(WorkspaceVisualNodeBase node)
+            private void ExecNpmInstallNew(string filePath)
             {
-                using (var npmController = this.CreateController(node.Workspace))
+                using (var npmController = this.CreateController(filePath))
                 using (var npmWorker = new NpmWorker(npmController))
                 using (var manager = new NpmPackageInstallWindow(npmController, npmWorker))
                 {
                     manager.ShowModal();
                 }
             }
 
-            private async void ExecNpmUpdate(WorkspaceVisualNodeBase node)
+            private async void ExecNpmUpdate(string filePath)
             {
-                using (var npmController = this.CreateController(node.Workspace))
+                using (var npmController = this.CreateController(filePath))
                 using (var commander = npmController.CreateNpmCommander())
                 {
                     await commander.UpdatePackagesAsync();
                 }
             }
 
-            private async void ExecDynamic(WorkspaceVisualNodeBase node, uint nCmdID)
+            private async void ExecDynamic(string filePath, uint nCmdID)
             {
                 // Unfortunately the NpmController (and NpmCommander), used for the install and update commands
                 // doesn't support running arbitrary scripts. And changing that is outside
                 // the scope of these changes.
-                var filePath = ((IFileNode)node).FullPath;
                 if (TryGetCommand(nCmdID, filePath, out var commandName))
                 {
-
-                    using (var npmController = this.CreateController(node.Workspace))
+                    using (var npmController = this.CreateController(filePath))
                     using (var commander = npmController.CreateNpmCommander())
                     {
                         await commander.ExecuteNpmCommandAsync($"run-script {commandName}", showConsole: true);
@@ -166,8 +167,13 @@ private async void ExecDebugAsync(WorkspaceVisualNodeBase node)
                 var workspace = node.Workspace;
                 var packageJson = PackageJsonFactory.Create(((IFileNode)node).FullPath);
 
+                if (string.IsNullOrEmpty(packageJson.Main))
+                {
+                    return;
+                }
+
                 //invoke debuglaunchtargetprovider on this file
-                var fileContextActions = await node.Workspace.GetFileContextActionsAsync(packageJson.Main, new[] { DebugLaunchActionContext.ContextTypeGuid });
+                var fileContextActions = await workspace.GetFileContextActionsAsync(packageJson.Main, new[] { DebugLaunchActionContext.ContextTypeGuid });
                 if (fileContextActions.Any())
                 {
                     // we requested a single context, so there should be a single grouping. Use the First action, since they're ordered by priority.
@@ -265,9 +271,12 @@ private static bool TryGetCommand(uint nCmdID, string filePath, out string comma
                 return false;
             }
 
-            private INpmController CreateController(IWorkspace workspace)
+            private INpmController CreateController(string packageJsonPath)
             {
-                var projectHome = workspace.Location;
+                Debug.Assert(Path.IsPathRooted(packageJsonPath));
+                Debug.Assert(PackageJsonFactory.IsPackageJsonFile(packageJsonPath));
+
+                var projectHome = Path.GetDirectoryName(packageJsonPath);
 
                 var npmController = NpmControllerFactory.Create(
                       projectHome,

Nodejs/Product/Nodejs/Workspace/PackageJsonScannerFactory.cs

@@ -43,13 +43,16 @@ protected override Task<List<FileReferenceInfo>> ComputeFileReferencesAsync(stri
 
                 var packageJson = PackageJsonFactory.Create(filePath);
                 var main = packageJson.Main;
-                var fileReferences = new List<FileReferenceInfo>
+
+                var fileReferences = new List<FileReferenceInfo>();
+
+                if (!string.IsNullOrEmpty(main))
                 {
-                    new FileReferenceInfo(main,
-                    context: "Debug",
-                    target: main,
-                    referenceType: (int)FileReferenceInfoType.Output)
-                };
+                    fileReferences.Add(new FileReferenceInfo(main,
+                        context: "Debug",
+                        target: main,
+                        referenceType: (int)FileReferenceInfoType.Output));
+                }
 
                 return Task.FromResult(fileReferences);
             }

Nodejs/Product/Nodejs/Workspace/TypeScriptActionProviderFactory.cs

@@ -60,7 +60,7 @@ public async Task<IReadOnlyList<IFileContextAction>> GetActionsAsync(string file
             {
                 await this.workspaceContext.JTF.SwitchToMainThreadAsync();
 
-                this.outputPane.ShowWindow();
+                this.outputPane.InitializeOutputPanes();
 
                 var actions = new List<IFileContextAction>();
 

Nodejs/Product/Npm/SPI/NpmCommand.cs

@@ -106,7 +106,7 @@ await NpmHelpers.ExecuteNpmCommandAsync(
                 cancelled = true;
             }
             OnCommandCompleted(this.Arguments, redirector?.HasErrors ?? false, cancelled);
-            return !redirector.HasErrors;
+            return redirector?.HasErrors != true; // return true when the command completed without errors, or redirector is null (which means we can't track)
         }
 
         private sealed class NpmCommandRedirector : Redirector