Rename --report-depth to --depth, keep legacy alias#101
Conversation
- Add --depth as primary CLI argument for markdown header depth - Keep --report-depth as legacy undocumented alias - Apply depth to self-validation markdown output in Validation.cs - Update README.md, user guide, design docs, requirements - Add new tests for --depth parameter (Context and CLI) - Keep legacy --report-depth tests - Add integration test for legacy alias - Update build.yaml to use --depth for sarifmark invocation Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
…s doc Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot Do you actually test that --depth affects the self-validation markdown report depth? |
Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/aec2e657-f77e-4870-a92a-6b4b9faaa78a Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Previously, Added |
Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/15f47b0f-78b6-4c5f-ac4b-f9560c16f86e Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR renames the CLI option for controlling markdown heading depth from --report-depth to --depth (keeping --report-depth as a legacy alias), and ensures that the selected depth is also applied during self-validation markdown report generation.
Changes:
- Update CLI parsing and internal API to use
Context.Depthas the canonical heading depth (with--report-depthretained as an alias). - Apply the configured depth to both normal markdown report generation and self-validation markdown report generation.
- Update tests, requirements, docs, and CI workflow usage to prefer
--depth.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.SarifMark.Tests/SelfTest/SelfTestTests.cs | Adds a self-test asserting --depth affects self-validation report generation outcomes. |
| test/DemaConsulting.SarifMark.Tests/ProgramTests.cs | Updates help output assertion to expect --depth. |
| test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs | Switches integration coverage to --depth and adds legacy alias acceptance coverage. |
| test/DemaConsulting.SarifMark.Tests/Cli/ContextTests.cs | Adds unit tests for --depth parsing/validation and updates legacy tests to assert Depth. |
| test/DemaConsulting.SarifMark.Tests/Cli/CliTests.cs | Adds subsystem test for --depth and updates legacy assertions to use Depth. |
| src/DemaConsulting.SarifMark/SelfTest/Validation.cs | Forwards --depth into the markdown report generation self-validation test and validates heading prefix accordingly. |
| src/DemaConsulting.SarifMark/Program.cs | Updates help text and report generation to use context.Depth. |
| src/DemaConsulting.SarifMark/Cli/Context.cs | Renames ReportDepth to Depth and parses --depth with a --report-depth alias. |
| docs/user_guide/introduction.md | Updates user documentation to --depth. |
| docs/reqstream/sarifmark/cli/context.yaml | Updates requirement text and adds --depth test references while retaining legacy coverage. |
| docs/reqstream/sarifmark/cli/cli.yaml | Updates CLI requirement to --depth and links new self-test. |
| docs/design/sarifmark/program.md | Updates design doc help flag listing to --depth. |
| docs/design/sarifmark/cli/context.md | Updates flag naming and legacy alias narrative (but see review comment re: property name). |
| docs/design/sarifmark/cli/cli.md | Updates flag naming (but see review comment re: property name). |
| README.md | Updates CLI options list to --depth. |
| .github/workflows/build.yaml | Updates workflow invocation to use --depth 1. |
Comments suppressed due to low confidence (2)
test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs:335
- The summary comment was updated to say "depth parameter", but the test method name is still
IntegrationTest_ReportDepth_IsConfigurable. Consider renaming the test toIntegrationTest_Depth_IsConfigurable(and updating any requirement links) to reflect the canonical flag name.
/// <summary>
/// Test that depth parameter is configurable.
/// </summary>
[TestMethod]
public void IntegrationTest_ReportDepth_IsConfigurable()
{
src/DemaConsulting.SarifMark/Program.cs:203
SarifResults.ToMarkdownthrowsArgumentOutOfRangeExceptionwhen depth is outside 1–6, but this report generation block doesn’t catch it (current catch filter excludes it). SinceContext.Depthonly validates>= 1, passing--depth 7will crash the program instead of producing a user-friendly error. Consider validatingDepthto be within 1–6 during argument parsing and/or catchingArgumentOutOfRangeExceptionhere and emitting a clear message (e.g., "--depth must be between 1 and 6").
context.WriteLine($"Writing report to {context.ReportFile}...");
try
{
var markdown = sarifResults.ToMarkdown(context.Depth, context.Heading);
File.WriteAllText(context.ReportFile, markdown);
context.WriteLine("Report generated successfully.");
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException)
{
context.WriteError($"Error: Failed to write report: {ex.Message}");
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
…ts/design docs Agent-Logs-Url: https://github.com/demaconsulting/SarifMark/sessions/3be1e216-163b-4795-a461-d8d05b38cd00 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Done in commit
|
Pull Request
Description
Renames
--report-depthto--depthas the canonical CLI argument.--report-depthremains functional as an undocumented legacy alias.--depthnow also controls the depth of self-validation markdown output (previously it had no effect there).Source changes
Context.cs—--depthis the primary case;--report-depthfalls through to the same handler;ReportDepthproperty renamed toDepthon both the outerContextclass and the innerArgumentParserclass; XML doc updated to "Gets the markdown depth."Program.cs— help text updated to--depth <depth>; usescontext.DepthValidation.cs— self-test runner now forwards--depth <context.Depth>so depth applies to self-validation outputTests
Cli_DepthParameter_SetsDepth(subsystem) and fourContext_Create_Depth_*(unit) tests for--depth--report-depthtests retained to verify backward compatibilityIntegrationTests.csprimary test uses--depth; added legacy acceptance testProgramTests.cshelp assertion updated to check--depthSelfTest_DepthParameter_AffectsSelfValidationReport— explicitly verifies that passing--depth 2causes the self-validation markdown report generation test to pass with depth-2 headings (##), confirming--depthactually affects the self-validation markdown outputcontext.DepthDocs & requirements
README.md,docs/user_guide/introduction.md, design docs updated to--depthcontext.mdandcli.mdupdated: property column renamed fromReportDepthtoDepthSelfTest_DepthParameter_AffectsSelfValidationReportlinked toSarifMark-Cli-ReportDepth;context.yamldescription updated fromset ReportDepthtoset Depth; test name references updated toCli_DepthParameter_SetsDepthandContext_Create_DepthParameter_SetsDepth--depth 1Type of Change
Related Issues
Pre-Submission Checklist
Before submitting this pull request, ensure you have completed the following:
Build and Test
dotnet build --configuration Releasedotnet test --configuration Releasedotnet run --project src/DemaConsulting.SarifMark --configuration Release --framework net10.0--no-build -- --validateCode Quality
dotnet format --verify-no-changesQuality Checks
Please run the following checks before submitting:
./lint.sh(Unix/macOS) orcmd /c lint.bat/./lint.bat(Windows)Testing
Documentation
Additional Notes
--report-depthis intentionally kept working but removed from all documentation. No behavior change for existing users. The internalReportDepthproperty has been renamed toDepthto align with the canonical CLI argument name. Test methods that exercise the--depthflag are named consistently with the property (SetsDepth), while legacy tests covering--report-depthretain their existing names.