Relax path validations (ignore %) (#2006)

DonJayamanne · web-flow · commit 488a1514c490 · 2018-06-19T14:28:23.000-07:00
* Relax path validations (ignore %)
* Allow trailing path delimiters
* Update message with solution
diff --git a/src/client/application/diagnostics/applicationDiagnostics.ts b/src/client/application/diagnostics/applicationDiagnostics.ts
@@ -27,7 +27,7 @@ export class ApplicationDiagnostics implements IApplicationDiagnostics {
         const logger = this.serviceContainer.get<ILogger>(ILogger);
         const outputChannel = this.serviceContainer.get<IOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
         diagnostics.forEach(item => {
-            const message = `Diagnostic Code: ${item.code}, Mesage: ${item.message}`;
+            const message = `Diagnostic Code: ${item.code}, Message: ${item.message}`;
             switch (item.severity) {
                 case DiagnosticSeverity.Error: {
                     logger.logError(message);
diff --git a/src/client/application/diagnostics/checks/envPathVariable.ts b/src/client/application/diagnostics/checks/envPathVariable.ts
@@ -16,8 +16,8 @@ import { DiagnosticCodes } from '../constants';
 import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../promptHandler';
 import { DiagnosticScope, IDiagnostic, IDiagnosticHandlerService } from '../types';
 
-const InvalidEnvPathVariableMessage = 'The environment variable \'{0}\' seems to have some paths containing characters (\';\', \'"\', \'%\' or \';;\').' +
-    ' The existence of such characters are known to have caused the {1} extension to not load.';
+const InvalidEnvPathVariableMessage = 'The environment variable \'{0}\' seems to have some paths containing characters (\';\', \'"\' or \';;\').' +
+    ' The existence of such characters are known to have caused the {1} extension to not load. If the extension fails to load please modify your paths to remove these characters.';
 
 export class InvalidEnvironmentPathVariableDiagnostic extends BaseDiagnostic {
     constructor(message) {
@@ -79,6 +79,6 @@ export class EnvironmentPathVariableDiagnosticsService extends BaseDiagnosticsSe
         const pathValue = currentProc.env[this.platform.pathVariableName];
         const pathSeparator = this.serviceContainer.get<IPathUtils>(IPathUtils).delimiter;
         const paths = pathValue.split(pathSeparator);
-        return paths.filter(item => item.indexOf('"') >= 0 || item.indexOf(';') >= 0 || item.indexOf('%') >= 0 || item.length === 0).length > 0;
+        return paths.filter((item, index) => item.indexOf('"') >= 0 || item.indexOf(';') >= 0 || (item.length === 0 && index !== paths.length - 1)).length > 0;
     }
 }
diff --git a/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts b/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts
@@ -79,7 +79,7 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => {
         }
 
         for (const diagnostic of diagnostics) {
-            const message = `Diagnostic Code: ${diagnostic.code}, Mesage: ${diagnostic.message}`;
+            const message = `Diagnostic Code: ${diagnostic.code}, Message: ${diagnostic.message}`;
             switch (diagnostic.severity) {
                 case DiagnosticSeverity.Error: {
                     logger.setup(l => l.logError(message))
diff --git a/src/test/application/diagnostics/checks/envPathVariable.unit.test.ts b/src/test/application/diagnostics/checks/envPathVariable.unit.test.ts
@@ -114,7 +114,8 @@ suite('Application Diagnostics - Checks Env Path Variable', () => {
 
         expect(diagnostics).to.be.deep.equal([]);
     });
-    [';;', '"', '%'].forEach(invalidCharacter => {
+    // Note: On windows, when a path contains a `;` then Windows encloses the path within `"`.
+    [';;', '"'].forEach(invalidCharacter => {
         test(`Should return single diagnostics for Windows if path contains ${invalidCharacter}`, async () => {
             platformService.setup(p => p.isWindows).returns(() => true);
             const paths = [
@@ -132,6 +133,18 @@ suite('Application Diagnostics - Checks Env Path Variable', () => {
             expect(diagnostics[0].severity).to.be.equal(DiagnosticSeverity.Warning);
             expect(diagnostics[0].scope).to.be.equal(DiagnosticScope.Global);
         });
+        test('Should not return diagnostics for Windows if path ends with delimiter', async () => {
+            const paths = [
+                path.join('one', 'two', 'three'),
+                path.join('one', 'two', 'four')
+            ].join(pathDelimiter) + pathDelimiter;
+            platformService.setup(p => p.isWindows).returns(() => true);
+            procEnv.setup(env => env[pathVariableName]).returns(() => paths);
+
+            const diagnostics = await diagnosticService.diagnose();
+
+            expect(diagnostics).to.be.lengthOf(0);
+        });
     });
     test('Should display three options in message displayed with 2 commands', async () => {
         platformService.setup(p => p.isWindows).returns(() => true);

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => {`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`for (const diagnostic of diagnostics) {`
`82`		- const message = `Diagnostic Code: ${diagnostic.code}, Mesage: ${diagnostic.message}`;
	`82`	+ const message = `Diagnostic Code: ${diagnostic.code}, Message: ${diagnostic.message}`;
`83`	`83`	`switch (diagnostic.severity) {`
`84`	`84`	`case DiagnosticSeverity.Error: {`
`85`	`85`	`logger.setup(l => l.logError(message))`