docs: multi-line block comment support design spec
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
# Multi-Line Block Comment Support — Design
|
||||
|
||||
## Problem
|
||||
|
||||
Comment removal is per-line and stateless (`removeCommentsFromLine`). Multi-line block comments (`/* ... */`, `<!-- ... -->`) are corrupted: only the first line is truncated at the start marker, and the comment body plus closing marker are left as live code, breaking compilation. This is worst in the auto-hook path (`shush --changes-only`), which rewrites files unattended with no backup.
|
||||
|
||||
Proven repros (both real runs, one compile-verified): a C license header and a straddling `/* ... */` both turn comment prose into code.
|
||||
|
||||
## Goal
|
||||
|
||||
Correctly remove multi-line block comments across every supported language that has them, via a shared stateful engine, without regressing single-line behavior (locked by the existing `TestWholeFile` golden suite).
|
||||
|
||||
## Decisions (locked)
|
||||
|
||||
1. **Coverage:** one block fixture per extension for all block-comment languages — 26 `/* */` extensions (js, ts, jsx, tsx, go, c, cpp, cc, cxx, h, hpp, java, cs, rs, swift, kt, kts, dart, scala, php, css, scss, sass, less, sql) and 4 `<!-- -->` extensions (html, htm, xml, svg). Mixed and edge-case scenarios use a representative subset (they test the algorithm, not per-extension wiring).
|
||||
2. **Git boundary:** skip straddling blocks. A block fully inside the changed range is removed; a block crossing a range boundary is left completely intact. Honors the "surgical, only changed lines" promise; never corrupts.
|
||||
3. **Multi-line strings** (Go raw strings, JS/TS template literals spanning lines with markers inside): documented non-goal for v1. String protection stays per-line.
|
||||
4. **Preview:** `--dry-run` preview is routed through the same engine so it never misreports multi-line blocks.
|
||||
|
||||
## Architecture
|
||||
|
||||
New file `internal/processor/engine.go` holds the single source of truth. It replaces the per-line `removeCommentsFromLine` as the transform used by all four call sites.
|
||||
|
||||
```go
|
||||
type lineResult struct {
|
||||
num int
|
||||
original string
|
||||
result string
|
||||
action string
|
||||
emit bool
|
||||
}
|
||||
|
||||
func (p *Processor) stripComments(lines []string, lang types.Language, cfg *config.Config, inRange func(lineNum int) bool) []lineResult
|
||||
```
|
||||
|
||||
- `action` is one of `"kept"`, `"removed"`, `"modified"`, `"preserved"`.
|
||||
- `emit == false` means the line is dropped from the output file (a fully-removed line, unless `--preserve-lines` turns it into a blank/leading-whitespace line, which is emitted).
|
||||
- `inRange == nil` means whole-file processing; otherwise it is a predicate over 1-based line numbers used by git mode.
|
||||
|
||||
Consumers:
|
||||
- `processFileInMemory` and `processFileWithLineRanges` (mutation): write `result` for every `emit` line.
|
||||
- `showPreview` and `showGitPreviewWithTotals` (display): render each line by `action`, count changed/kept/preserved.
|
||||
|
||||
## Algorithm
|
||||
|
||||
Scan line by line, carrying block state via lookahead (a block is resolved to its end before deciding, so preservation and the straddle rule can see the whole block).
|
||||
|
||||
For each line, outside of strings, find the leftmost meaningful token:
|
||||
|
||||
- **String open** (`"`, `'`, `` ` ``): skip to its matching close on the same line, respecting `\` escapes. If it does not close on the line, the rest of the line is treated as string and state resets on the next line (per-line string handling, status quo).
|
||||
- **Line-comment marker** (processed only when not `--block`): cut from the marker to end of line. Because this is chosen only when it is the leftmost token, a `/*` appearing after `//` is inert (inside the line comment).
|
||||
- **Block-start marker** (skipped entirely when `--inline`): resolve the matching end marker via lookahead across following lines to form a block region, then decide:
|
||||
- **Straddle rule:** if `inRange != nil` and any line of the block is outside the range, keep the whole block verbatim.
|
||||
- **Preserve:** accumulate the full block text; if `cfg.ShouldPreserveComment` matches, keep the whole block verbatim.
|
||||
- **Remove:** keep code before the start marker on the first line and code after the end marker on the last line; drop interior lines. Start and end on the same line collapse to `prefix + suffix`. An unterminated block (no end by EOF) removes from the start marker to EOF.
|
||||
|
||||
Line comments in git mode are gated per line: strip only if that line satisfies `inRange`.
|
||||
|
||||
`--preserve-lines`: a line that becomes empty due to comment removal (including block interior lines) is emitted as its leading whitespace instead of being dropped.
|
||||
|
||||
## Edge-case semantics
|
||||
|
||||
| Input | Result | Reason |
|
||||
|-------|--------|--------|
|
||||
| `/* a // b */` | removed | `//` inside a block is not a line comment |
|
||||
| `x // a /* b` | `x` | `/*` inside a line comment is inert |
|
||||
| `x /* a */ // b` | `x` | block removed, then line comment removed |
|
||||
| `code /* c` / `body` / `*/ y()` | `code` / `y()` | multi-line block; boundary code kept |
|
||||
| `/* no end` / `more` | both removed | unterminated block runs to EOF |
|
||||
| `*/ x()` (no open) | unchanged | no open block; not a comment token |
|
||||
| `s = "/* not */"` | unchanged | marker inside a string is protected |
|
||||
|
||||
## Non-goals (v1)
|
||||
|
||||
- Multi-line strings spanning lines with comment markers inside.
|
||||
- Nested block comments (C-family block comments do not nest).
|
||||
- `--dry-run` preview line-number remapping beyond what `action` rendering needs.
|
||||
|
||||
## Testing (TDD)
|
||||
|
||||
Sequence: write fixtures and failing tests, run and observe red, build the engine and rewire call sites, run and observe green, then `make check`.
|
||||
|
||||
- **Block, every extension (~30):** `testdata/multiline/block_<ext>.<ext>` plus `.golden`, for all 26 `/* */` and 4 `<!-- -->` extensions. Each fixture is a multi-line block with code before and after it.
|
||||
- **Mixed (representative — C, Go, CSS, HTML):** interleaved line and block comments in one file.
|
||||
- **Edge cases (representative):** one fixture per row of the edge-case table — marker-in-block, marker-in-line-comment, block-then-line, unterminated, stray close, string-protected, boundary code.
|
||||
- **Git straddle (JS):** a range fully inside a block is removed; a straddling block is kept. Uses `processFileWithLineRanges` with explicit `[]git.LineRange` (no real git).
|
||||
- **Regression:** every existing `TestWholeFile` single-line golden must stay green.
|
||||
|
||||
Driver reuses the existing golden harness pattern (copy input to a temp dir, run the mutate path, byte-compare against `.golden`), extended for the multi-line fixtures.
|
||||
|
||||
## Constraints
|
||||
|
||||
- No code comments anywhere (hard project rule).
|
||||
- Version control: jj only.
|
||||
- Go 1.24, module path `github.com/carlosarraes/shush`.
|
||||
- Gate: `make check` (fmt + vet + test) clean before every commit.
|
||||
Reference in New Issue
Block a user