build.ps1/build_helpers.ps1 issues

Open
#169 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
35/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Stale
Tech stack
powershell

Research direction

Read build.ps1 and tools/build-helpers.ps1, starting with ParseJasmine and RunSpecs, then review ExtractAtom and the build entry points against the linked lines. Done means the listed help, attribute, invocation, path, and parsing issues are addressed without breaking the build or Jasmine output.

Written by the indexing model from the issue text.

Description

Issue Description
build.ps1
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

https://github.com/PowerShell/EditorSyntax/blob/cf27d6e54ff9fc593bfb46d22a671898fb31c0a9/tools/build-helpers.ps1#L38-L56

  • 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:
    https://github.com/PowerShell/EditorSyntax/blob/cf27d6e54ff9fc593bfb46d22a671898fb31c0a9/tools/build-helpers.ps1#L73
    to read more like:
            & $script:ATOM_EXE_PATH --test $specpath *>&1 | ParseJasmine
    
function RunSpecs

I'll post a PR shortly demonstrating these changes.

Dominant language
PowerShell
Stars
151
Forks
55
PR merge metrics
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PowerShell/EditorSyntax

All issues in PowerShell/EditorSyntax

Similar issues

More Build System issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.