Skip to content

Commit e776106

Browse files
authored
Merge pull request #60 from jakehildreth:docs/document-new-features
docs(phase-7): document ConvertTo-StepperScript, retry behavior, and fix dialog option casing
2 parents 0cd183a + 3da99f9 commit e776106

43 files changed

Lines changed: 282 additions & 224 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Docs/adr/0001-ast-parsing-over-regex.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Three categories of fragility were identified:
1414

1515
1. **Structural detection:** `New-Step` regex requires `{` on the same line as the call, breaking if the scriptblock is on the next line. Manual brace-counting to find block end does not account for `{`/`}` inside strings or heredocs.
1616
2. **Comment span detection:** `<#` / `#>` are matched line-by-line, which breaks if either token appears after executable code on the same line.
17-
3. **Executable-line classification:** A 5-pattern regex whitelist (`[CmdletBinding(`, `param(`, `using`, etc.) is used to skip "non-executable" lines. Incomplete by construction any pattern not in the list is a false positive.
17+
3. **Executable-line classification:** A 5-pattern regex whitelist (`[CmdletBinding(`, `param(`, `using`, etc.) is used to skip "non-executable" lines. Incomplete by construction; any pattern not in the list is a false positive.
1818

1919
Additionally, the step-counting + name-listing logic is copy-pasted verbatim 3 times in `New-Step.ps1` (lines 349–371, 501–529, 713–740), and the script file is read from disk 2–3 times per `New-Step` execution.
2020

@@ -27,7 +27,7 @@ Replace all structural regex with `[System.Management.Automation.Language.Parser
2727
Introduce a new private function `Get-ScriptAst` that:
2828
- Calls `[Parser]::ParseFile()`, returning `[PSCustomObject]@{ Ast; Tokens; Errors }`
2929
- Emits `Write-Warning` per parse error and returns the partial AST (callers decide whether to abort)
30-
- Maintains a module-scoped `$script:astCache` hashtable keyed on `"$ScriptPath:$hash"` (using the existing `Get-ScriptHash` function) auto-invalidates when the script changes, eliminates redundant disk reads
30+
- Maintains a module-scoped `$script:astCache` hashtable keyed on `"$ScriptPath:$hash"` (using the existing `Get-ScriptHash` function); auto-invalidates when the script changes, eliminates redundant disk reads
3131

3232
---
3333

@@ -39,14 +39,14 @@ The PowerShell AST is syntax-aware. It correctly handles:
3939
- `New-Step` and `Stop-Stepper` appearing inside comments (they simply won't produce `CommandAst` nodes)
4040
- All PS quoting rules by construction
4141

42-
The alternative keeping regex but fixing edge cases is a local patch on a structural problem. Every edge case fixed reveals the next one.
42+
The alternative (keeping regex but fixing edge cases) is a local patch on a structural problem. Every edge case fixed reveals the next one.
4343

4444
---
4545

4646
## Consequences
4747

4848
- `Get-ScriptAst.ps1` added to `Private/`
49-
- `Get-StepInventory.ps1` added to `Private/` encapsulates the deduplicated step-counting logic
49+
- `Get-StepInventory.ps1` added to `Private/`: encapsulates the deduplicated step-counting logic
5050
- `Find-NewStepBlocks.ps1` signature changes from `[object[]]$ScriptLines` to `[string]$ScriptPath`; brace-counting loop replaced with `$ast.FindAll(...)` + `Extent` properties
5151
- `Find-UnmanagedCodeBlocks.ps1` comment detection replaced with token stream inspection; executable-line classification replaced with AST extent check
5252
- `Test-StepperScriptRequirements.ps1` uses `$ast.ParamBlock.Attributes` and `$ast.ScriptRequirements.RequiredModules` instead of regex
@@ -58,6 +58,6 @@ The alternative — keeping regex but fixing edge cases — is a local patch on
5858

5959
## Rejected alternative
6060

61-
**Direct `[Parser]::ParseFile()` calls per function** each function would handle its own `[ref]` vars and error handling. Rejected because:
61+
**Direct `[Parser]::ParseFile()` calls per function**: each function would handle its own `[ref]` vars and error handling. Rejected because:
6262
- Error handling (`Write-Warning` per parse error) would be copy-pasted into 4–5 functions and drift over time
63-
- The re-parsing problem (2–3 disk reads per execution) stays unfixed caching can only be centralized in a wrapper
63+
- The re-parsing problem (2–3 disk reads per execution) stays unfixed; caching can only be centralized in a wrapper

Docs/adr/0002-logging-and-step-transcripts.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
## Context
1010

11-
Stepper had no persistent log output. All status information was emitted via `Write-Verbose`, which is ephemeral lost if the caller omits `-Verbose` or does not capture output. There was no record of step runtimes, step failures, or what happened during a previous run.
11+
Stepper had no persistent log output. All status information was emitted via `Write-Verbose`, which is ephemeral; lost if the caller omits `-Verbose` or does not capture output. There was no record of step runtimes, step failures, or what happened during a previous run.
1212

1313
Three design questions required explicit decisions before implementation:
1414

@@ -24,13 +24,13 @@ Three design questions required explicit decisions before implementation:
2424

2525
Logging is enabled for every step unless the user explicitly passes `-NoLog` to a `New-Step` call. When `-NoLog` is present on any step, Stepper prompts the user at init time to choose scope:
2626

27-
- **A** log all steps (ignore `-NoLog` flags)
28-
- **S** skip logging for the flagged steps only
29-
- **D** disable logging entirely
27+
- **A**, log all steps (ignore `-NoLog` flags)
28+
- **S**, skip logging for the flagged steps only
29+
- **D**, disable logging entirely
3030

3131
This decision is persisted in the `.stepper` state file so resumed runs do not re-prompt.
3232

33-
Rationale: unattended scripts need logs most requiring an explicit opt-in means logs are missing exactly when they're most needed.
33+
Rationale: unattended scripts need logs most; requiring an explicit opt-in means logs are missing exactly when they're most needed.
3434

3535
### Native `Start-Transcript` per step, folded into the log file
3636

@@ -42,7 +42,7 @@ Rationale: `Start-Transcript` captures the full host output including `Write-Hos
4242

4343
Before starting a per-step transcript, Stepper checks `$Host.UI.IsTranscribing`. If `$true`, it emits a clear user-facing message and throws a terminating error (`TranscriptAlreadyActive`, category `ResourceBusy`). It does not silently skip transcript capture and continue.
4444

45-
Rationale: silently skipping produces a log with missing transcript sections worse than a clear failure because the user may not notice. A hard stop with an actionable message (`Stop-Transcript and re-run`) is unambiguous.
45+
Rationale: silently skipping produces a log with missing transcript sections; worse than a clear failure because the user may not notice. A hard stop with an actionable message (`Stop-Transcript and re-run`) is unambiguous.
4646

4747
### Default log path: `scriptname.ps1.stepper.log` alongside the script
4848

@@ -54,19 +54,19 @@ If one or more `New-Step` calls specify `-LogPath` with a static string, that pa
5454

5555
`LogPath`, `LoggingEnabled`, and `NoLogStepIds` are added to the existing `Export-Clixml` state object. On a resumed run, these values are read from state and applied without re-prompting.
5656

57-
Rationale: consistency the path and scope decisions made on the first run should apply to the resumed run, which may be unattended.
57+
Rationale: consistency; the path and scope decisions made on the first run should apply to the resumed run, which may be unattended.
5858

5959
---
6060

6161
## Consequences
6262

6363
- **New private functions:** `Write-StepperLog` (structured log helper), `Get-StepLogConfig` (AST scan for `-LogPath` and `-NoLog` across all steps)
64-
- **State schema change:** three new fields (`LogPath`, `LoggingEnabled`, `NoLogStepIds`) added to the `Export-Clixml` object in `Write-StepperState.ps1`. Backwards compatible old state files simply lack these fields (treated as `$null`/`$false`).
64+
- **State schema change:** three new fields (`LogPath`, `LoggingEnabled`, `NoLogStepIds`) added to the `Export-Clixml` object in `Write-StepperState.ps1`. Backwards compatible; old state files simply lack these fields (treated as `$null`/`$false`).
6565
- **`New-Step` new params:** `-LogPath [string]`, `-NoLog [switch]`
6666
- **`Stop-Stepper` no new params:** reads `LogPath` from `__StepperExecutionState` in the calling scope; no user-facing change.
67-
- **Temp files:** one temp file per step execution, deleted in a `finally` block. If the process is killed mid-step, the temp file may be orphaned in `$env:TEMP` acceptable.
67+
- **Temp files:** one temp file per step execution, deleted in a `finally` block. If the process is killed mid-step, the temp file may be orphaned in `$env:TEMP`; acceptable.
6868
- **PS 5.1 + active transcript:** hard stop with `TranscriptAlreadyActive`. Users running enterprise runbooks that pre-start transcripts must stop them before using Stepper logging. `-NoLog` on all steps or `-N` at the scope prompt is the workaround.
69-
- **`Read-Host` not captured in transcripts on Unix/macOS:** `Start-Transcript` on PS Core (Unix/macOS) does not capture `Read-Host` prompts or their responses. Pipeline output, `Write-Host`, and `Write-Output` are captured normally. This is a `Start-Transcript` platform limitation not a Stepper defect. On Windows (PS 5.1 or PS 7), `Read-Host` prompts are captured.
69+
- **`Read-Host` not captured in transcripts on Unix/macOS:** `Start-Transcript` on PS Core (Unix/macOS) does not capture `Read-Host` prompts or their responses. Pipeline output, `Write-Host`, and `Write-Output` are captured normally. This is a `Start-Transcript` platform limitation; not a Stepper defect. On Windows (PS 5.1 or PS 7), `Read-Host` prompts are captured.
7070

7171
---
7272

Docs/adr/0003-silent-auto-add-requirements.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212

1313
This behavior is problematic:
1414

15-
1. **Blocks automation** any CI/CD pipeline or unattended run hangs indefinitely waiting for interactive input.
16-
2. **Surprises users** an unexpected halt mid-execution on first run violates the principle of least surprise.
17-
3. **Violates module design principles** `Read-Host` is explicitly against Stepper's non-interactive design pattern.
18-
4. **"Skip" is a footgun** silently continuing without the required declarations causes unpredictable failures downstream, making it the worst option despite being the easiest to reach.
15+
1. **Blocks automation**: any CI/CD pipeline or unattended run hangs indefinitely waiting for interactive input.
16+
2. **Surprises users**: an unexpected halt mid-execution on first run violates the principle of least surprise.
17+
3. **Violates module design principles**: `Read-Host` is explicitly against Stepper's non-interactive design pattern.
18+
4. **"Skip" is a footgun**: silently continuing without the required declarations causes unpredictable failures downstream, making it the worst option despite being the easiest to reach.
1919

2020
The only safe response in the old dialog was always "Add". The "Skip" and "Quit" paths offer false choice.
2121

@@ -27,14 +27,14 @@ Replace the interactive prompt with silent auto-add behavior:
2727

2828
- If declarations are missing, add them and return `$true` without any user prompt.
2929
- Emit one `Write-Verbose` call per declaration added.
30-
- Delete any existing state file (same as before the script has structurally changed).
30+
- Delete any existing state file (same as before; the script has structurally changed).
3131
- Add a `-SkipRequirementsCheck` `[switch]` parameter to `New-Step` for callers who want to bypass the check entirely.
3232

3333
---
3434

3535
## Rationale
3636

37-
Silent auto-add preserves the original intent (ensure the script is properly structured) while eliminating interactive friction. Since "Add" was the only safe option in the old prompt, auto-add is semantically equivalent to the old default. `Write-Verbose` satisfies observability without blocking execution users running with `-Verbose` see exactly what changed.
37+
Silent auto-add preserves the original intent (ensure the script is properly structured) while eliminating interactive friction. Since "Add" was the only safe option in the old prompt, auto-add is semantically equivalent to the old default. `Write-Verbose` satisfies observability without blocking execution; users running with `-Verbose` see exactly what changed.
3838

3939
The `-SkipRequirementsCheck` escape hatch gives advanced users who consciously manage their own declarations a clean opt-out, without requiring workarounds or file hacks.
4040

Docs/adr/0004-convert-steppersscript-output-target.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
`Convert-StepperScript` rewrites variable references inside `New-Step` blocks from `$var` to `$Stepper.Var`. A rewrite tool that modifies files needs a clear answer to "where does the output go?"
1212

1313
Two options were considered:
14-
1. **In-place only** always overwrites the source file
15-
2. **In-place with backup + optional -OutputPath** default is in-place with a `.bak` copy; `-OutputPath` writes to a new file instead, leaving the original untouched
14+
1. **In-place only**: always overwrites the source file
15+
2. **In-place with backup + optional -OutputPath**: default is in-place with a `.bak` copy; `-OutputPath` writes to a new file instead, leaving the original untouched
1616

1717
Option 1 is simpler but gives users no recovery path if the rewrite produces unexpected results. A `.bak` is trivial to create and provides a safety net without any extra cognitive overhead.
1818

@@ -29,7 +29,7 @@ Option 2 gives maximum flexibility: users who want a diff-friendly workflow can
2929
## Rationale
3030

3131
- In-place is the expected UX for a "migration" tool (lower friction)
32-
- `.bak` is zero-cost insurance most users will never need it, but they'll be grateful when they do
32+
- `.bak` is zero-cost insurance; most users will never need it, but they'll be grateful when they do
3333
- `-OutputPath` enables review workflows and CI dry-run scenarios without a separate `-WhatIf` level of complexity
3434
- Consistent with how other script-rewriting tools in the ecosystem behave (e.g. `2to3`, `ps-upgrade`)
3535

@@ -40,4 +40,4 @@ Option 2 gives maximum flexibility: users who want a diff-friendly workflow can
4040
- `Convert-StepperScript` has a common `-OutputPath [string]` optional parameter
4141
- When `-OutputPath` is absent: reads source, writes `.bak`, writes rewritten content back to source
4242
- When `-OutputPath` is present: reads source, writes rewritten content to `-OutputPath`, source untouched, no `.bak`
43-
- `-WhatIf` support via `SupportsShouldProcess` prints what would change without writing anything
43+
- `-WhatIf` support via `SupportsShouldProcess`; prints what would change without writing anything

Docs/adr/0005-cbh-injection-is-silent.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
`Repair-StepperScript` (and the first-run hook in `New-Step`) detects whether a user's script has comment-based help (CBH). If CBH is absent, Stepper can inject a skeleton block automatically.
1212

1313
Two injection styles were considered:
14-
1. **Silent** inject without prompting; emit one `Write-Verbose` call; continue execution
15-
2. **Interactive** print a notice, call `exit`, require user to re-run (same pattern as `MissingCmdletBinding` in the old `Test-StepperScriptRequirements`)
14+
1. **Silent**: inject without prompting; emit one `Write-Verbose` call; continue execution
15+
2. **Interactive**: print a notice, call `exit`, require user to re-run (same pattern as `MissingCmdletBinding` in the old `Test-StepperScriptRequirements`)
1616

1717
The `[CmdletBinding()]` and install-guard injections are already silent as of ADR 0003. CBH injection modifies the script in the same category of "structural improvement" as those two, and carries no functional risk (CBH is inert at runtime).
1818

@@ -30,7 +30,7 @@ CBH injection is silent. No `Write-Host`, no `exit`, no user prompt. Consistent
3030

3131
- CBH is documentation-only; its absence or presence does not affect script execution
3232
- Interrupting execution for a non-functional change violates the principle of least surprise
33-
- Consistency with existing silent injections (ADR 0003) users learn one mental model
33+
- Consistency with existing silent injections (ADR 0003); users learn one mental model
3434
- Verbose output is still emitted, so `-Verbose` users see what happened
3535

3636
---

Docs/adr/0006-new-steppersscript-showcase-switch.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@
1111
`New-StepperScript` generates a new `.ps1` file pre-wired for Stepper. The question was how much content to include by default.
1212

1313
Two extremes:
14-
1. **Minimal skeleton** just the structural requirements: CBH, `[CmdletBinding()] param()`, install guard, two placeholder `New-Step` blocks, `Stop-Stepper`. Minimal noise, immediately editable.
15-
2. **Feature-showcase template** everything in minimal, plus commented examples demonstrating named steps, `$Stepper.<var>` persistence, `-NoLog`, `-Retry`/`-RetryInterval`/`-MaxRetries`, and `#region Stepper ignore` blocks.
14+
1. **Minimal skeleton**: just the structural requirements: CBH, `[CmdletBinding()] param()`, install guard, two placeholder `New-Step` blocks, `Stop-Stepper`. Minimal noise, immediately editable.
15+
2. **Feature-showcase template**: everything in minimal, plus commented examples demonstrating named steps, `$Stepper.<var>` persistence, `-NoLog`, `-Retry`/`-RetryInterval`/`-MaxRetries`, and `#region Stepper ignore` blocks.
1616

17-
Both templates can be useful in either situation the right choice depends on intent, not experience level. A single fixed template forces a tradeoff: a minimal-only tool requires users to look up API details elsewhere; a showcase-only tool clutters files that just need a starting point.
17+
Both templates can be useful in either situation; the right choice depends on intent, not experience level. A single fixed template forces a tradeoff: a minimal-only tool requires users to look up API details elsewhere; a showcase-only tool clutters files that just need a starting point.
1818

1919
The solution is a default of minimal with an opt-in flag for the full showcase.
2020

@@ -28,9 +28,9 @@ The solution is a default of minimal with an opt-in flag for the full showcase.
2828

2929
## Rationale
3030

31-
- Minimal is the right default because the common case is starting a new script, not exploring the API less to delete, immediately writable
31+
- Minimal is the right default because the common case is starting a new script, not exploring the API; less to delete, immediately writable
3232
- Showcase is the right opt-in for any time you want in-file examples as a reference, regardless of experience level
33-
- Aliases make the flag discoverable users who think `-Full` or `-WithExamples` are likely to get tab-completion
33+
- Aliases make the flag discoverable; users who think `-Full` or `-WithExamples` are likely to get tab-completion
3434
- Single implementation path per template variant (no runtime branching inside each `New-Step` block)
3535

3636
---

Docs/adr/0007-new-steppersscript-parameter-sets.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
`New-StepperScript` needs a way to specify where to write the new script. Two calling styles exist in the wild:
1212

13-
- `New-StepperScript -Path './scripts/Deploy.ps1'` caller specifies a full path
14-
- `New-StepperScript -Name 'Deploy' -Directory './scripts'` caller specifies name and location separately; the function constructs the path
13+
- `New-StepperScript -Path './scripts/Deploy.ps1'`: caller specifies a full path
14+
- `New-StepperScript -Name 'Deploy' -Directory './scripts'`: caller specifies name and location separately; the function constructs the path
1515

1616
Both are idiomatic for PowerShell creation functions. `New-Item` uses `-Path`. `New-ADUser` and similar functions use `-Name` + a location parameter. Some users will have a full path from a variable; others want to type less by specifying just a name.
1717

@@ -23,7 +23,7 @@ Supporting only one style forces awkward usage for the other half of callers.
2323

2424
`New-StepperScript` uses two parameter sets:
2525

26-
- **`ByPath`**: `-Path [string]` (mandatory) full file path
26+
- **`ByPath`**: `-Path [string]` (mandatory): full file path
2727
- **`ByName`**: `-Name [string]` (mandatory) + `-Directory [string]` (optional, defaults to `$PWD`)
2828

2929
Both produce identical output; `ByName` constructs the full path as `Join-Path $Directory "$Name.ps1"`.

0 commit comments

Comments
 (0)