build.ps1/build_helpers.ps1 issues

[msftrncs]

Issue Description

build.ps1

• comment based help attempt to include an argument to the example statement, this causes PowerShell to invalidate the comment based help entirely.
  

  EditorSyntax/build.ps1

  Line 10 in cf27d6e

  .EXAMPLE Build

• [cmdletbinding()] and [parameter()] attributes are unnecessary.
  

  EditorSyntax/build.ps1

  Lines 28 to 33 in cf27d6e

  	[cmdletbinding()]
  	param(
  	[parameter()]
  	[switch]
  	$Test
  	)

• use of & (call operator) to execute commands with an unquoted (and non-expanded) name are unnecessary. (3 occurrences total)
  

  EditorSyntax/build.ps1

  Line 41 in cf27d6e

  if (-not(& npm -v)) {

tools\build_helpers.ps1

function ExtractAtom

• doesn't support UNC paths due to $PSScriptRoot and Resolve-Path will format with the filesystem:: provider, which notation is not acceptable to [System.IO.Compression.ZipFile]::ExtractToDirectory() method. This is informational only, due to the fact that UNC paths do not work for NPM anyway.

function ParseJasmine

EditorSyntax/tools/build-helpers.ps1

Lines 38 to 56 in cf27d6e

	function ParseJasmine ($String) {
	if ($String -match "^\s+at.*$") {
	' '
	return
	}
	if ($_ -match "^\s+it.*$") {
	$_ -replace '^(\s+)(it)','$1[-] It'
	return
	}
	if ($_ -match "^\s+Expected.*$") {
	$x = $_ -replace '^(\s*)(Expected)(.*)\s(to equal .*)','$1$2$3%%$1$4'
	$x.Replace('%%',"`n ")
	return
	}
	$_
	}

• first line starts with using $string parameter, the rest use $_. Surprisingly it works, but only because in RunSpecs the call to ParseJasmine is part of a ForEach-Object which populates the $_ automatic variable.
• the ^\s+Expected replacement doesn't seem to be working correctly. Maybe Atom/Atom-Grammar-Test has changed? I think 'to equal' now is to be 'instead found'.
• this function should really be a filter as it might provide a better fit. Note, also demonstrating the 'switch' statement with -regex parameter:

  filter ParseJasmine {
      switch -regex ($_) {
          ^\s+at {
              ''
              break
          }
  
          ^\s+it {
              $_ -replace '^(\s+)(it)', '$1[-] It'
              break
          }
  
          ^\s+Expected {
              $_ -replace '^(\s*)(Expected.*?)\s(instead found .*)', "`$1`$2`n`$1`$3"
              break
          }
  
          default {
              $_
          }
      }
  }

  This requires changing RunSpecs:
  

  EditorSyntax/tools/build-helpers.ps1

  Line 73 in cf27d6e

  & "$script:ATOM_EXE_PATH" --test spec *>&1 | ForEach-Object { ParseJasmine $_ }

  
  to read more like:

          & $script:ATOM_EXE_PATH --test $specpath *>&1 | ParseJasmine

function RunSpecs

• quotes around the variable $script:ATOM_EXE_PATH are unneeded, probably old habit from CMD or another shell? (seen below)
• test path 'specs' is literal on ATOM command, but yet in a variable $specpath in a test previously. Only issue with using the variable is a chance to get the PowerShell filesystem:: provider notation injected.
  

  EditorSyntax/tools/build-helpers.ps1

  Lines 72 to 73 in cf27d6e

  	if (Test-Path $specpath) {
  	& "$script:ATOM_EXE_PATH" --test spec *>&1 | ForEach-Object { ParseJasmine $_ }

I'll post a PR shortly demonstrating these changes.

[msftrncs]

Realized I missed a minor detail:

Originally:

$x = $_ -replace '^(\s*)(Expected)(.*)\s(to equal .*)','$1$2$3%%$1$4' 
$x.Replace('%%',"`n  ") 

I replaced with:

$_ -replace '^(\s*)(Expected.*?)\s(instead found .*)', "`$1`$2`n`$1`$3"

I missed the two spaces following the `n in the string .replace() method invocation. Ideally these should be stuck between `$1 and `$3 incase $1 results in a pattern of `t. They cause the instead found message to be slightly indented compared to the Expected message, but I am not sure this is too important.