Add Markdown output support #2344
Conversation
- Introduce `MarkDownOutput` flag and CLI option (`-markdown` / `-md`). - Implement `Result.MarkdownOutput` to generate a formatted Markdown table with optional title and CDN columns, and a response body preview. - Add markdown escaping helper. - Update runner to create `.md` output file, handle markdown generation per result, and write to file or stdout. - Adjust option parsing and related logic to include markdown handling.
WalkthroughAdds Markdown table output: new header/row renderers in Changes
Sequence Diagram(s)sequenceDiagram
%%{init: {"themeVariables": {"actorBorder":"#374151","actorBackground":"#F8FAFC","noteBackground":"#EEF2FF"}}}%%
participant Runner
participant MDFile as "MD File\n(filesystem)"
participant Stdout
participant Result as "Result → MarkdownRow"
Runner->>MDFile: Open Output + ".md" (if MarkDownOutput)
Note right of MDFile: file handle created\nmarkdownHeaderWritten=false
Runner->>MDFile: Write MarkdownHeader (once)
Runner->>Stdout: Optionally print header
loop per response
Runner->>Result: Build MarkdownRow(scanopts)
Result-->>Runner: MarkdownRow string
Runner->>MDFile: Append MarkdownRow
Runner->>Stdout: Optionally print row
end
Runner->>MDFile: Close file (deferred on exit)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
runner/md_output.go (1)
61-67: Consider additional Markdown escaping for edge cases.The current implementation escapes pipes and newlines, which covers the primary cases for table cell content. However, consider whether these additional characters should be escaped:
- Backticks
`(could break inline code blocks if used in title)- Square brackets
[and](could create unintended links)For the current use case (table cells), the existing escaping is likely sufficient, but be aware of potential edge cases.
runner/runner.go (1)
845-845: Consider renamingjsonOrCsvvariable for clarity.The variable now includes Markdown output but retains the name
jsonOrCsv, which is misleading. Consider renaming to something likestructuredOutputorjsonOrCsvOrMarkdownfor better maintainability.This is used on lines 847 and 1105 to control output behavior, so the rename would affect those locations as well.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
runner/md_output.gorunner/options.gorunner/runner.go
🔇 Additional comments (5)
runner/options.go (2)
225-225: LGTM!The
MarkDownOutputfield is properly positioned alongside other output format flags and follows existing conventions.
482-482: LGTM!The CLI flag binding is properly configured and follows the same pattern as other output format flags.
runner/runner.go (3)
818-818: LGTM!The
mdFilevariable declaration follows the existing pattern for other output file handles.
835-843: LGTM!The markdown file opening logic correctly follows the pattern established for JSON and CSV outputs, including proper resume support and cleanup.
1241-1249: Markdown output integration is correctly implemented.The markdown output logic properly integrates with the existing output flow and mirrors the JSON/CSV patterns. However, note the table structure issue flagged in
runner/md_output.go: each result generates its own complete table rather than contributing rows to a unified table.Unlike CSV (lines 889-897), which writes one header then rows for all results, the current markdown implementation writes a complete table (header + separator + row) per result. This is correctly implemented given the current design of
MarkdownOutput(), but results in multiple separate tables in the output file.Suggestion: If a unified table is desired (similar to CSV), consider writing a markdown table header once at the beginning (similar to CSV header on lines 889-897), then have
MarkdownOutput()return only the data row without header/separator.
- Split Markdown generation into `MarkdownHeader` and `MarkdownRow` for clearer separation. - Remove body‑preview block from Markdown rows. - Add guard in runner to write the header only once per run.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
runner/md_output.go (2)
9-31: Table structure still inconsistent: conditional columns break Markdown rendering.The header conditionally includes Title and CDN columns based on a single Result instance's field values. This creates mismatched tables when different results have different fields populated.
Problem scenario:
- First result has
Title=""→ header generated with 7 columns (no Title column)- Second result has
Title="Hello"→ row written with 8 cells (includes Title)- Result: broken table with mismatched column counts
The split into MarkdownHeader/MarkdownRow (addressing the past review comment) solves the repeated-header issue but not the column consistency problem.
To ensure consistent table structure, always include Title and CDN columns regardless of whether individual results have those values populated. Write empty cells when values are blank.
🔎 Proposed fix to always include Title and CDN columns
func MarkdownHeader(r Result) string { var b strings.Builder - b.WriteString("| URL | Status | Method | IP | Size | Words | Lines |") - if r.Title != "" { - b.WriteString(" Title |") - } - if r.CDNName != "" { - b.WriteString(" CDN |") - } + b.WriteString("| URL | Status | Method | IP | Size | Words | Lines | Title | CDN |") b.WriteString("\n") - b.WriteString("|---|---|---|---|---|---|---|") - if r.Title != "" { - b.WriteString("---|") - } - if r.CDNName != "" { - b.WriteString("---|") - } + b.WriteString("|---|---|---|---|---|---|---|---|---|") b.WriteString("\n") return b.String() }Then update MarkdownRow to always write Title and CDN cells (see separate comment).
33-54: Critical: URL field not escaped; conditional cells break table structure.Two issues in this function:
The URL field (line 37) is not escaped, which will break the table if it contains pipe characters (e.g.,
https://example.com?param=value|other). This was flagged in the past review comment and remains unresolved.Conditional Title and CDN cells (lines 45-50) create mismatched column counts when different results have different field values. Must always write these cells (empty if blank) to match the header's column structure.
🔎 Proposed fix for URL escaping and consistent cells
func (r Result) MarkdownRow(scanopts *ScanOptions) string { var b strings.Builder fmt.Fprintf(&b, "| %s | `%d %s` | `%s` | `%s` | %d | %d | %d |", - r.URL, + escapeMarkdown(r.URL), r.StatusCode, http.StatusText(r.StatusCode), r.Method, r.HostIP, r.ContentLength, r.Words, r.Lines) - if r.Title != "" { + if r.Title != "" { fmt.Fprintf(&b, " %s |", escapeMarkdown(r.Title)) + } else { + b.WriteString(" |") } - if r.CDNName != "" { + if r.CDNName != "" { fmt.Fprintf(&b, " `%s` |", r.CDNName) + } else { + b.WriteString(" |") } b.WriteString("\n") return b.String() }
🧹 Nitpick comments (1)
runner/md_output.go (1)
33-33: Consider removing or documenting the unusedscanoptsparameter.The
scanoptsparameter is declared but never used in the function body. If it's reserved for future functionality (e.g., conditionally formatting fields based on scan options), consider adding a comment explaining its purpose. Otherwise, remove it to keep the signature clean.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runner/md_output.gorunner/runner.go
🚧 Files skipped from review as they are similar to previous changes (1)
- runner/runner.go
🧰 Additional context used
🧬 Code graph analysis (1)
runner/md_output.go (1)
runner/types.go (1)
Result(35-105)
🔇 Additional comments (1)
runner/md_output.go (1)
56-62: LGTM: Basic escaping logic is sound.The function correctly escapes pipe characters and replaces newlines, which are the primary concerns for Markdown table structure. The main issue is that it's not being applied to the URL field (covered in the previous comment).
…d` flag * Consolidate output‑type checks into a single `jsonOrCsvOrMD` variable. * Adjust file‑opening logic to add the “.md” suffix only when needed. * Update related conditionals to respect the new combined flag handling. * Ensure markdown output is correctly written without creating “.md.md” files.
1 participant
MarkDownOutputflag and CLI option (-markdown/-md).Result.MarkdownOutputto generate a formatted Markdown table with optional title and CDN columns, and a response body preview..mdoutput file, handle markdown generation per result, and write to file or stdout.Preview:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.