Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enables strict mode testing of module, fix git restore --source= tab bug #783

Merged
merged 2 commits into from
Aug 3, 2020
Merged

Enables strict mode testing of module, fix git restore --source= tab bug #783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c80348
Enables strict mode testing of module, fix git restore --source= tab bug
rkeithhill Jul 26, 2020
0d2f471
Can't access non-existing props in strict mode
rkeithhill Jul 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion PSScriptAnalyzerSettings.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# Use ExcludeRules when you want to run most of the default set of rules except
# for a few rules you wish to "exclude". Note: if a rule is in both IncludeRules
# and ExcludeRules, the rule will be excluded.
ExcludeRules = @('PSAvoidUsingWriteHost', 'PSAvoidGlobalVars', 'PSAvoidUsingInvokeExpression')
ExcludeRules = @('PSAvoidUsingWriteHost', 'PSAvoidGlobalVars', 'PSAvoidUsingInvokeExpression', 'PSReviewUnusedParameter')

# You can use the following entry to supply parameters to rules that take parameters.
# For instance, the PSAvoidUsingCmdletAliases rule takes a whitelist for aliases you
Expand Down
8 changes: 4 additions & 4 deletions src/GitTabExpansion.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ function script:gitFeatures($filter, $command) {
$branches = @(git branch --no-color | ForEach-Object { if ($_ -match "^\*?\s*$featurePrefix(?<ref>.*)") { $matches['ref'] } })
$branches |
Where-Object { $_ -ne '(no branch)' -and $_ -like "$filter*" } |
ForEach-Object { $prefix + $_ } |
ForEach-Object { $featurePrefix + $_ } |
quoteStringWithSpecialChars
}

Expand Down Expand Up @@ -390,9 +390,9 @@ function GitTabExpansionInternal($lastBlock, $GitStatus = $null) {
gitCheckoutFiles $GitStatus $matches['files']
}

# Handles git restore -s <ref> - must come before the next regex case
"^restore.* (?-i)-s\s*(?<ref>\S*)$" {
gitBranches $matches['ref'] $true
# Handles git restore -s <ref> / --source=<ref> - must come before the next regex case
"^restore.* (?-i)(-s\s*|(?<source>--source=))(?<ref>\S*)$" {
gitBranches $matches['ref'] $true $matches['source']
gitTags $matches['ref']
break
}
Expand Down
10 changes: 7 additions & 3 deletions src/GitUtils.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,11 @@ function Get-GitBranch($gitDir = $(Get-GitDirectory), [Diagnostics.Stopwatch]$sw
}

dbg 'Inside git directory?' $sw
if ('true' -eq $(git rev-parse --is-inside-git-dir 2>$null)) {
$revParseOut = git rev-parse --is-inside-git-dir 2>$null
if ('true' -eq $revParseOut) {
dbg 'Inside git directory' $sw
if ('true' -eq $(git rev-parse --is-bare-repository 2>$null)) {
$revParseOut = git rev-parse --is-bare-repository 2>$null
if ('true' -eq $revParseOut) {
$c = 'BARE:'
}
else {
Expand Down Expand Up @@ -251,6 +253,8 @@ function Get-GitStatus {
$aheadBy = 0
$behindBy = 0
$gone = $false
$upstream = $null

$indexAdded = New-Object System.Collections.Generic.List[string]
$indexModified = New-Object System.Collections.Generic.List[string]
$indexDeleted = New-Object System.Collections.Generic.List[string]
Expand Down Expand Up @@ -317,7 +321,7 @@ function Get-GitStatus {
switch ($settings.UntrackedFilesMode) {
"No" { $untrackedFilesOption = "-uno" }
"All" { $untrackedFilesOption = "-uall" }
"Normal" { $untrackedFilesOption = "-unormal" }
default { $untrackedFilesOption = "-unormal" }
}
$status = Invoke-Utf8ConsoleCommand { git -c core.quotepath=false -c color.status=false status $untrackedFilesOption --short --branch 2>$null }
if ($settings.EnableStashStatus) {
Expand Down
9 changes: 7 additions & 2 deletions src/posh-git.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
param([bool]$ForcePoshGitPrompt, [bool]$UseLegacyTabExpansion)

if (Test-Path Env:\POSHGIT_ENABLE_STRICTMODE) {
# Set strict mode to latest to help catch scripting errors in the module. This is done by the Pester tests.
Set-StrictMode -Version Latest
}
Comment on lines +3 to +6
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume Set-StrictMode is best done here rather than in test/Shared.ps1 to scope strictness to the module, rather than all tests too?

Hi, hello, yes it's me I still exist. Barely. 👋

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I found when I set it in Shared.ps1 that A) it complained about some of our "lose" scripting in the tests and B) it wasn't finding issues in the module. That goes counter to the claim in #758 but it makes sense that enabling set-strictmode in the module just for testing is the way to go - IMO.

BTW I don't think we want to leave strict mode on for ship because it will gen errors with user injected code like this:

$global:GitPromptSettings.DefaultPromptSuffix.Text = ' $((Get-History -Count 1).id + 1)$(">" * ($nestedPromptLevel + 1)) '

For example, when you first start PowerShell there is no history so Get-HIstory returns $null and strict-mode doesn't like the null de-ref to get the id. But for my prompt string it works out - $null + 1 is 1.


. $PSScriptRoot\CheckRequirements.ps1 > $null

. $PSScriptRoot\ConsoleMode.ps1
Expand All @@ -16,8 +21,8 @@ param([bool]$ForcePoshGitPrompt, [bool]$UseLegacyTabExpansion)
$IsAdmin = Test-Administrator

# Get the default prompt definition.
$initialSessionState = [Runspace]::DefaultRunspace.InitialSessionState
if (!$initialSessionState.Commands -or !$initialSessionState.Commands['prompt']) {
$initialSessionState = [System.Management.Automation.Runspaces.Runspace]::DefaultRunspace.InitialSessionState
if (!$initialSessionState -or !$initialSessionState.PSObject.Properties.Match('Commands') -or !$initialSessionState.Commands['prompt']) {
$defaultPromptDef = "`$(if (test-path variable:/PSDebugContext) { '[DBG]: ' } else { '' }) + 'PS ' + `$(Get-Location) + `$(if (`$nestedpromptlevel -ge 1) { '>>' }) + '> '"
}
else {
Expand Down
21 changes: 19 additions & 2 deletions test/Shared.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,23 @@
# Define these variables since they are not defined in WinPS 5.x
if ($PSVersionTable.PSVersion.Major -lt 6) {
[System.Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssigments', '')]
$IsWindows = $true
[System.Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssigments', '')]
$IsLinux = $false
[System.Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssigments', '')]
$IsMacOS = $false
}

$modulePath = Convert-Path $PSScriptRoot\..\src
$moduleManifestPath = "$modulePath\posh-git.psd1"

[System.Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssigments', '')]
$csi = [char]0x1b + "["

[System.Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssigments', '')]
$expectedEncoding = if ($PSVersionTable.PSVersion.Major -le 5) { "utf8" } else { "ascii" }

[System.Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssigments', '')]
$originalTitle = $Host.UI.RawUI.WindowTitle

if (!(Get-Variable -Name gitbin -Scope global -ErrorAction SilentlyContinue)) {
Expand Down Expand Up @@ -137,7 +152,9 @@ function ResetGitTempRepoWorkingDir($RepoPath, $Branch = 'master') {
Remove-Item Function:\prompt
Remove-Module posh-git -Force *>$null

# Force the posh-git prompt to be installed. Could be runnng on dev system where
# user has customized the prompt.
# For Pester testing, enable strict mode inside the posh-git module
$env:POSHGIT_ENABLE_STRICTMODE = 1

# Force the posh-git prompt to be installed. Could be runnng on dev system where user has customized the prompt.
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssigments', '')]
$module = Import-Module $moduleManifestPath -ArgumentList $true,$false -Force -PassThru