|
| 1 | +# Plan Spec: List Spacing Control (`--list-spacing` Option) |
| 2 | + |
| 3 | +## Purpose |
| 4 | + |
| 5 | +This is a technical design doc for adding a `--list-spacing` CLI option to Flowmark that |
| 6 | +controls how tight vs loose list formatting is handled during Markdown normalization. |
| 7 | + |
| 8 | +**Terminology Note**: CommonMark uses "tight" and "loose" as the official terms for list |
| 9 | +spacing. A tight list has no blank lines between items; a loose list has blank lines. |
| 10 | + |
| 11 | +## Background |
| 12 | + |
| 13 | +Flowmark has historically converted **all lists to loose format** (blank lines between |
| 14 | +items). This was an intentional design decision documented in |
| 15 | +[markdown_filling.py:51-52](src/flowmark/linewrapping/markdown_filling.py#L51-L52): |
| 16 | + |
| 17 | +> "Also enforces that all list items have two newlines between them, so that items are |
| 18 | +> separate paragraphs when viewed as plaintext." |
| 19 | +
|
| 20 | +**CommonMark Spec Reference:** |
| 21 | + |
| 22 | +Per [CommonMark 0.31.2](https://spec.commonmark.org/0.31.2/#lists): |
| 23 | + |
| 24 | +| List Type | Definition | HTML Rendering | |
| 25 | +|-----------|------------|----------------| |
| 26 | +| **Tight** | No blank lines between items | Content NOT wrapped in `<p>` tags | |
| 27 | +| **Loose** | Blank lines between items OR internal blank lines | Content wrapped in `<p>` tags | |
| 28 | + |
| 29 | +A list becomes loose if: |
| 30 | +1. Any blank line separates consecutive items |
| 31 | +2. Any item contains multiple block elements with blank lines between them |
| 32 | + |
| 33 | +**Current Behavior Issues:** |
| 34 | + |
| 35 | +- Cannot round-trip documents (tight lists become loose) |
| 36 | +- Author intent for compact lists is not preserved |
| 37 | +- HTML rendering changes semantically (`<li>text</li>` vs `<li><p>text</p></li>`) |
| 38 | +- Files become larger due to extra newlines |
| 39 | + |
| 40 | +**Key Finding:** |
| 41 | + |
| 42 | +Marko (the Markdown parser) exposes a `tight` boolean attribute on parsed `List` elements, |
| 43 | +making it feasible to detect and preserve the original list style. |
| 44 | + |
| 45 | +## Summary of Task |
| 46 | + |
| 47 | +Add a `--list-spacing` CLI option with three modes: |
| 48 | + |
| 49 | +| Mode | Behavior | |
| 50 | +|------|----------| |
| 51 | +| `preserve` | Keep lists tight or loose as authored (NEW DEFAULT) | |
| 52 | +| `loose` | Convert all lists to loose format (current behavior) | |
| 53 | +| `tight` | Convert all lists to tight format where possible | |
| 54 | + |
| 55 | +This will be exposed as: |
| 56 | +- CLI: `--list-spacing={preserve|tight|loose}` (default: `preserve`) |
| 57 | +- API: `list_spacing: Literal["preserve", "tight", "loose"] = "preserve"` parameter |
| 58 | + |
| 59 | +## Backward Compatibility |
| 60 | + |
| 61 | +**BACKWARD COMPATIBILITY REQUIREMENTS:** |
| 62 | + |
| 63 | +- **Code types, methods, and function signatures**: KEEP DEPRECATED - existing callers |
| 64 | + using default parameters should get the new `preserve` behavior, but we need to ensure |
| 65 | + no breaking changes for anyone explicitly passing parameters |
| 66 | + |
| 67 | +- **Library APIs**: KEEP DEPRECATED for `reformat_text()` and `fill_markdown()` - add new |
| 68 | + `list_spacing` parameter with default that maintains current output for most use cases |
| 69 | + |
| 70 | +- **CLI behavior**: BREAKING CHANGE (intentional) - the new default `preserve` will |
| 71 | + produce different output for tight lists. This is the desired behavior per user request. |
| 72 | + |
| 73 | +- **File formats**: SUPPORT BOTH - existing Markdown files should produce similar output |
| 74 | + for already-loose lists; tight lists will now remain tight by default |
| 75 | + |
| 76 | +**Key Compatibility Note:** |
| 77 | + |
| 78 | +The `testdoc.expected.*.md` files will need to be regenerated since they contain tight |
| 79 | +lists that are currently converted to loose. The new expected output will preserve tight |
| 80 | +lists. |
| 81 | + |
| 82 | +## Stage 1: Planning Stage |
| 83 | + |
| 84 | +### Minimum Viable Feature |
| 85 | + |
| 86 | +1. Add `--list-spacing` CLI option with `preserve`, `loose`, and `tight` modes |
| 87 | +2. Default to `preserve` (breaking change from current `loose` behavior) |
| 88 | +3. Detect list tightness via Marko's `list.tight` attribute |
| 89 | +4. Thread spacing mode through to list rendering |
| 90 | +5. Update test expectations |
| 91 | + |
| 92 | +### Not In Scope |
| 93 | + |
| 94 | +- Per-list overrides (e.g., `<!-- flowmark: loose -->` comments) |
| 95 | +- Heuristic-based auto-detection of "should be tight" vs "should be loose" |
| 96 | +- Different behavior for nested lists vs top-level lists |
| 97 | + |
| 98 | +### Acceptance Criteria |
| 99 | + |
| 100 | +1. `flowmark --list-spacing=preserve file.md` preserves tight lists as tight |
| 101 | +2. `flowmark --list-spacing=preserve file.md` preserves loose lists as loose |
| 102 | +3. `flowmark --list-spacing=loose file.md` converts all lists to loose (current behavior) |
| 103 | +4. `flowmark --list-spacing=tight file.md` converts all lists to tight where possible |
| 104 | +5. `flowmark file.md` uses `preserve` by default |
| 105 | +6. Nested lists are controlled independently (each preserves its own tightness) |
| 106 | +7. Lists with multi-paragraph items remain loose regardless of mode (CommonMark requirement) |
| 107 | +8. `make lint` and `make test` pass |
| 108 | + |
| 109 | +### Resolved Questions |
| 110 | + |
| 111 | +- [x] **Q1**: Should `tight` mode force-convert loose lists, or only convert lists that |
| 112 | + CAN be tight (no multi-paragraph items)? |
| 113 | + **Decision**: Only convert lists that can be tight; leave multi-paragraph items loose. |
| 114 | + |
| 115 | +- [x] **Q2**: How should nested lists behave? Same mode as parent, or independent? |
| 116 | + **Decision**: Each list independently uses the mode—nested tight stays tight, nested |
| 117 | + loose stays loose in `preserve` mode. |
| 118 | + |
| 119 | +- [x] **Q3**: Should `--auto` flag continue to use `loose`, or switch to `preserve`? |
| 120 | + **Decision**: All modes use `preserve` as the default. Explicit `--list-spacing=X` |
| 121 | + overrides any other settings (including `--auto`). |
| 122 | + |
| 123 | +## Stage 2: Architecture Stage |
| 124 | + |
| 125 | +### Current Architecture |
| 126 | + |
| 127 | +List rendering is handled in |
| 128 | +[flowmark_markdown.py:226-261](src/flowmark/formats/flowmark_markdown.py#L226-L261): |
| 129 | + |
| 130 | +```python |
| 131 | +def render_list(self, element: block.List) -> str: |
| 132 | + # Currently ignores element.tight |
| 133 | + result: list[str] = [] |
| 134 | + for i, child in enumerate(element.children): |
| 135 | + # ... configure prefix ... |
| 136 | + with self.container(prefix, subsequent_indent): |
| 137 | + rendered_item = self.render(child) |
| 138 | + result.append(rendered_item) |
| 139 | + return "".join(result) |
| 140 | + |
| 141 | +def render_list_item(self, element: block.ListItem) -> str: |
| 142 | + result = "" |
| 143 | + # We want all list items to have two newlines between them. |
| 144 | + if self._suppress_item_break: |
| 145 | + self._suppress_item_break = False |
| 146 | + else: |
| 147 | + result += self._second_prefix.strip() + "\n" # <-- Always adds blank line |
| 148 | + result += self.render_children(element) |
| 149 | + return result |
| 150 | +``` |
| 151 | + |
| 152 | +**Key Issue**: `render_list_item()` always adds a blank line between items via |
| 153 | +`_suppress_item_break` logic, regardless of the original list tightness. |
| 154 | + |
| 155 | +### Proposed Changes |
| 156 | + |
| 157 | +#### Change 1: Add `ListSpacing` Enum |
| 158 | + |
| 159 | +**Location**: New in `flowmark_markdown.py` or separate types module |
| 160 | + |
| 161 | +```python |
| 162 | +from enum import StrEnum |
| 163 | + |
| 164 | +class ListSpacing(StrEnum): |
| 165 | + preserve = "preserve" |
| 166 | + loose = "loose" |
| 167 | + tight = "tight" |
| 168 | +``` |
| 169 | + |
| 170 | +#### Change 2: Thread Spacing Mode Through Renderer |
| 171 | + |
| 172 | +**Location**: `MarkdownNormalizer.__init__()` and `flowmark_markdown()` |
| 173 | + |
| 174 | +Add `_list_spacing: ListSpacing` instance variable that controls list rendering behavior. |
| 175 | + |
| 176 | +#### Change 3: Modify `render_list()` to Detect Tightness |
| 177 | + |
| 178 | +**Location**: `render_list()` method |
| 179 | + |
| 180 | +```python |
| 181 | +def render_list(self, element: block.List) -> str: |
| 182 | + # Determine effective tightness based on mode |
| 183 | + if self._list_spacing == ListSpacing.preserve: |
| 184 | + is_tight = element.tight |
| 185 | + elif self._list_spacing == ListSpacing.tight: |
| 186 | + is_tight = self._can_be_tight(element) # Check for multi-para items |
| 187 | + else: # loose |
| 188 | + is_tight = False |
| 189 | + |
| 190 | + # Pass is_tight to render_list_item via instance variable |
| 191 | + old_tight = self._current_list_tight |
| 192 | + self._current_list_tight = is_tight |
| 193 | + # ... existing rendering logic ... |
| 194 | + self._current_list_tight = old_tight |
| 195 | +``` |
| 196 | + |
| 197 | +#### Change 4: Modify `render_list_item()` to Respect Tightness |
| 198 | + |
| 199 | +**Location**: `render_list_item()` method |
| 200 | + |
| 201 | +```python |
| 202 | +def render_list_item(self, element: block.ListItem) -> str: |
| 203 | + result = "" |
| 204 | + # Only add blank line for loose lists |
| 205 | + if not self._current_list_tight: |
| 206 | + if self._suppress_item_break: |
| 207 | + self._suppress_item_break = False |
| 208 | + else: |
| 209 | + result += self._second_prefix.strip() + "\n" |
| 210 | + # ... rest of method |
| 211 | +``` |
| 212 | + |
| 213 | +#### Change 5: Update CLI and API |
| 214 | + |
| 215 | +**Locations**: |
| 216 | +- `cli.py`: Add `--list-spacing` argument |
| 217 | +- `reformat_api.py`: Add `list_spacing` parameter to `reformat_text()`, `reformat_file()`, |
| 218 | + `reformat_files()` |
| 219 | +- `markdown_filling.py`: Add `list_spacing` parameter to `fill_markdown()` |
| 220 | + |
| 221 | +### Architecture Diagram |
| 222 | + |
| 223 | +``` |
| 224 | +CLI: --list-spacing=preserve |
| 225 | + │ |
| 226 | + ▼ |
| 227 | + reformat_file() |
| 228 | + │ |
| 229 | + ▼ |
| 230 | + fill_markdown(list_spacing="preserve") |
| 231 | + │ |
| 232 | + ▼ |
| 233 | + flowmark_markdown(line_wrapper, list_spacing="preserve") |
| 234 | + │ |
| 235 | + ▼ |
| 236 | + MarkdownNormalizer(line_wrapper, list_spacing="preserve") |
| 237 | + │ |
| 238 | + ├─► render_list() ──► reads element.tight |
| 239 | + │ │ |
| 240 | + │ ▼ |
| 241 | + │ Determines effective tightness based on mode |
| 242 | + │ │ |
| 243 | + │ ▼ |
| 244 | + └─► render_list_item() ──► adds blank line only if not tight |
| 245 | +``` |
| 246 | + |
| 247 | +## Stage 3: Implementation Phases |
| 248 | + |
| 249 | +### Phase 1: Core Infrastructure |
| 250 | + |
| 251 | +- [ ] Add `ListSpacing` enum |
| 252 | +- [ ] Add `list_spacing` parameter to `MarkdownNormalizer.__init__()` |
| 253 | +- [ ] Add `_current_list_tight` instance variable for threading state |
| 254 | +- [ ] Add `_can_be_tight()` helper method for `tight` mode |
| 255 | + |
| 256 | +### Phase 2: List Rendering Changes |
| 257 | + |
| 258 | +- [ ] Modify `render_list()` to compute effective tightness |
| 259 | +- [ ] Modify `render_list_item()` to conditionally add blank lines |
| 260 | +- [ ] Handle edge cases (nested lists, code blocks in items, etc.) |
| 261 | + |
| 262 | +### Phase 3: API Updates |
| 263 | + |
| 264 | +- [ ] Add `list_spacing` parameter to `fill_markdown()` |
| 265 | +- [ ] Add `list_spacing` parameter to `reformat_text()` |
| 266 | +- [ ] Add `list_spacing` parameter to `reformat_file()` and `reformat_files()` |
| 267 | + |
| 268 | +### Phase 4: CLI Updates |
| 269 | + |
| 270 | +- [ ] Add `--list-spacing` argument to CLI |
| 271 | +- [ ] Update `--auto` flag to use `preserve` |
| 272 | +- [ ] Update help text |
| 273 | + |
| 274 | +### Phase 5: Testing and Documentation |
| 275 | + |
| 276 | +- [ ] Add unit tests for `ListSpacing` modes |
| 277 | +- [ ] Update `test_list_spacing.py` with preserve/tight mode tests |
| 278 | +- [ ] Regenerate `testdoc.expected.*.md` files |
| 279 | +- [ ] Update README/docstrings |
| 280 | + |
| 281 | +## Stage 4: Validation Stage |
| 282 | + |
| 283 | +### Test Strategy |
| 284 | + |
| 285 | +1. **Unit Tests**: Test each spacing mode with tight and loose input lists |
| 286 | +2. **Edge Cases**: |
| 287 | + - Nested lists (tight outer, loose inner and vice versa) |
| 288 | + - Lists with code blocks (must remain loose per CommonMark) |
| 289 | + - Lists with multiple paragraphs (must remain loose) |
| 290 | + - Mixed content lists |
| 291 | +3. **Integration Tests**: Full document processing with various modes |
| 292 | +4. **Regression Tests**: Ensure loose mode produces identical output to current behavior |
| 293 | + |
| 294 | +### Test Cases |
| 295 | + |
| 296 | +```python |
| 297 | +# Tight list stays tight in preserve mode |
| 298 | +def test_tight_list_preserved(): |
| 299 | + input = "- one\n- two\n- three" |
| 300 | + output = fill_markdown(input, list_spacing="preserve") |
| 301 | + assert output == "- one\n- two\n- three\n" |
| 302 | + |
| 303 | +# Tight list becomes loose in loose mode |
| 304 | +def test_tight_list_to_loose(): |
| 305 | + input = "- one\n- two\n- three" |
| 306 | + output = fill_markdown(input, list_spacing="loose") |
| 307 | + assert output == "- one\n\n- two\n\n- three\n" |
| 308 | + |
| 309 | +# Loose list becomes tight in tight mode |
| 310 | +def test_loose_list_to_tight(): |
| 311 | + input = "- one\n\n- two\n\n- three" |
| 312 | + output = fill_markdown(input, list_spacing="tight") |
| 313 | + assert output == "- one\n- two\n- three\n" |
| 314 | + |
| 315 | +# Multi-paragraph item stays loose even in tight mode |
| 316 | +def test_multi_para_stays_loose(): |
| 317 | + input = "- para1\n\n para2\n- item2" |
| 318 | + output = fill_markdown(input, list_spacing="tight") |
| 319 | + # Item with multiple paragraphs forces loose |
| 320 | + assert "\n\n" in output |
| 321 | +``` |
| 322 | + |
| 323 | +### Success Criteria |
| 324 | + |
| 325 | +- [ ] All existing tests pass (with updated expectations) |
| 326 | +- [ ] New spacing mode tests pass |
| 327 | +- [ ] `make lint` passes |
| 328 | +- [ ] `make test` passes |
| 329 | +- [ ] Manual testing with real documents confirms expected behavior |
0 commit comments