Skip to content

Fix whitespace tooling edge cases #777

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

Open
johnml1135 wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −4
Open

Fix whitespace tooling edge cases #777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion Build/Agent/check-and-fix-whitespace.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
#!/usr/bin/env pwsh
$ErrorActionPreference = 'Continue'

if (Test-Path -LiteralPath 'check-results.log') {
Remove-Item -LiteralPath 'check-results.log' -Force
}

& "$PSScriptRoot/check-whitespace.ps1"
$ec = $LASTEXITCODE

& "$PSScriptRoot/fix-whitespace.ps1"
if ($ec -eq 0 -or (Test-Path -LiteralPath 'check-results.log')) {
& "$PSScriptRoot/fix-whitespace.ps1"
}
else {
Write-Error 'Whitespace checker failed before producing results; skipping fixer.'
}
Comment on lines +11 to +16
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Using only file existence as a proxy for 'checker produced results' becomes less reliable now that the checker may create an empty check-results.log as a placeholder. To better match the intent ('skip fixer after failures without results'), consider checking for non-empty results (e.g., file length > 0) or using a more explicit success/results signal from the checker.

Copilot uses AI. Check for mistakes.

exit $ec
7 changes: 5 additions & 2 deletions Build/Agent/check-whitespace.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ Write-Host "Base ref: $baseRef ($baseSha)"
$log = git log --check --pretty=format:'---% h% s' "$baseSha.." 2>&1
$null = $log | Tee-Object -FilePath check-results.log
$log | Out-Host
if (-not (Test-Path -LiteralPath 'check-results.log')) {
New-Item -ItemType File -Path 'check-results.log' -Force | Out-Null
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Use -LiteralPath for New-Item here (to match Test-Path -LiteralPath and avoid provider/path parsing surprises). This keeps path handling consistent and reduces edge-case failures with special characters.

Suggested change
New-Item -ItemType File -Path 'check-results.log' -Force | Out-Null
New-Item -ItemType File -LiteralPath 'check-results.log' -Force | Out-Null

Copilot uses AI. Check for mistakes.
}

$problems = New-Object System.Collections.Generic.List[string]
$commit = ''
Expand All @@ -46,7 +49,7 @@ $commitTextmd = ''
$repoPath = Get-RepoPath
$headRef = (git rev-parse --abbrev-ref HEAD 2>$null)

Get-Content check-results.log | ForEach-Object {
$log | ForEach-Object {
$line = $_
switch -regex ($line) {
'^---\s' {
Expand Down Expand Up @@ -83,7 +86,7 @@ Get-Content check-results.log | ForEach-Object {
}

if ($problems.Count -gt 0) {
Write-Host "`u26A0`uFE0F Please review the output for further information."
Write-Host '[WARN] Please review the output for further information.'
Write-Host '### A whitespace issue was found in one or more of the commits.'
Write-Host 'This check validates commit history from origin/main..HEAD, not just the current working tree.'
Write-Host 'If the report names an older commit, fix the file and then amend, squash, or rebase so that commit no longer appears in the branch history.'
Expand Down
2 changes: 1 addition & 1 deletion Build/Agent/fix-whitespace.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ if (Test-Path -LiteralPath 'check-results.log') {
if (-not $fixFiles -or $fixFiles.Count -eq 0) {
$base = Get-BaseRef
Write-Host "Fixing whitespace for files changed since $base..HEAD"
$fixFiles = git diff --name-only "$base"..HEAD
$fixFiles = git diff --name-only $base HEAD
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Changing from git diff <base>..<head> semantics to git diff <base> <head> can alter which changes/files are selected (notably when the base ref is not a strict ancestor, since A..B uses the merge-base). If the intent is still 'files changed since base', consider passing a single revision-range argument (e.g., \"$base..HEAD\") rather than two separate commit arguments.

Suggested change
$fixFiles = git diff --name-only $base HEAD
$fixFiles = git diff --name-only "$base..HEAD"

Copilot uses AI. Check for mistakes.
}

$files = $fixFiles | Where-Object { $_ -and (Test-Path $_) }
Expand Down
Loading