From 4516c50accbb2f13dd73f37de9ad5989ef7b7499 Mon Sep 17 00:00:00 2001 From: Myriad-Dreamin Date: Thu, 23 Apr 2026 07:53:11 +0800 Subject: [PATCH] Extract folding range pipeline into standalone change --- .../improve-folding-range-support/design.md | 266 ------------------ .../improve-folding-range-support/proposal.md | 33 --- .../specs/folding-ranges/spec.md | 65 ----- .../improve-folding-range-support/tasks.md | 39 --- .../.openspec.yaml | 2 +- .../split-folding-range-pipeline/design.md | 104 +++++++ .../split-folding-range-pipeline/proposal.md | 26 ++ .../specs/folding-range-pipeline/spec.md | 38 +++ .../split-folding-range-pipeline/tasks.md | 16 ++ 9 files changed, 185 insertions(+), 404 deletions(-) delete mode 100644 openspec/changes/improve-folding-range-support/design.md delete mode 100644 openspec/changes/improve-folding-range-support/proposal.md delete mode 100644 openspec/changes/improve-folding-range-support/specs/folding-ranges/spec.md delete mode 100644 openspec/changes/improve-folding-range-support/tasks.md rename openspec/changes/{improve-folding-range-support => split-folding-range-pipeline}/.openspec.yaml (50%) create mode 100644 openspec/changes/split-folding-range-pipeline/design.md create mode 100644 openspec/changes/split-folding-range-pipeline/proposal.md create mode 100644 openspec/changes/split-folding-range-pipeline/specs/folding-range-pipeline/spec.md create mode 100644 openspec/changes/split-folding-range-pipeline/tasks.md diff --git a/openspec/changes/improve-folding-range-support/design.md b/openspec/changes/improve-folding-range-support/design.md deleted file mode 100644 index acdbf018..00000000 --- a/openspec/changes/improve-folding-range-support/design.md +++ /dev/null @@ -1,266 +0,0 @@ -## Context - -`clice` currently implements folding ranges in `src/Feature/FoldingRange.cpp`. The implementation is primarily an AST visitor with extra handling for conditional compilation and `#pragma region` data from `CompilationUnitRef::directives()`. It already covers many structural folds that clangd does not currently expose, such as namespaces, records, function parameter lists, lambda captures, call argument lists, access-specifier sections, and initializer lists. - -The request path is currently split across: - -- `src/Feature/FoldingRange.cpp` for collection -- `src/Server/Feature.cpp` for LSP rendering -- `src/Server/Lifecycle.cpp` for capability advertisement -- `include/Protocol/Feature/FoldingRange.h` for protocol types -- `tests/unit/Feature/FoldingRangeTests.cpp` for unit coverage - -That split reveals three immediate shortcomings: - -- the server always emits `kind = "region"` and always sets `collapsedText` -- folding-specific client capabilities are modeled in protocol types but not consumed by the server -- directive-related tests are mostly placeholders and do not assert important behavior - -The comparison target for this branch should be fixed and versioned. At tag `llvmorg-21.1.8`, clangd's folding behavior is centered on `clang-tools-extra/clangd/SemanticSelection.cpp`, with request plumbing in `ClangdServer.cpp` and `ClangdLSPServer.cpp`, protocol types in `Protocol.h` and `Protocol.cpp`, and regression coverage in `test/folding-range.test` plus folding-related unit tests. The branch should vendor that focused source snapshot first so implementation discussions can refer to committed upstream code instead of informal recollection. - -Compared with that clangd baseline, the current gap is clear: - -- clangd already has behavior that `clice` still lacks: - - multiline comment folding - - contiguous `//` comment-group folding - - `lineFoldingOnly` rendering behavior - - consistent use of standard public folding kinds - - a more complete and assertion-backed folding-range test matrix -- `clice` already has behavior that clangd does not: - - richer AST-structure folding - - `#pragma region` and some conditional-compilation folding - - `collapsedText` -- `clice` still has obvious opportunities that are not fully implemented yet: - - fully closing the last `#if/#elif/#else` branch at `#endif` - - folding inactive branches - - folding multiline macro definitions - - grouping contiguous `#include` / `import` blocks - - capability-aware `kind` and `collapsedText` rendering - -In addition, the current public output exposes many internal categories directly through `FoldingRange.kind`, such as `"namespace"`, `"class"`, and `"functionBody"`. This is more aggressive than clangd, but standard LSP only defines `comment`, `imports`, and `region` as interoperable folding kinds. The design therefore needs to preserve richer internal classification while presenting a compatible external contract. - -## Goals / Non-Goals - -**Goals:** - -- Vendor a focused clangd reference snapshot from `llvmorg-21.1.8` and use it as the explicit comparison baseline for this branch. -- Preserve `clice`'s current advantage in AST-structure folding instead of regressing to clangd's much narrower block-only baseline. -- Fill the high-value baseline gaps that clangd already covers, especially multiline comments and `lineFoldingOnly`. -- Turn preprocessor metadata into a differentiating `clice` capability covering conditional branches, macro definitions, and include/import grouping. -- Make folding-range output respect client capabilities with predictable fallback behavior. -- Lock behavior down with unit and integration tests across AST, comments, preprocessor handling, and protocol negotiation. - -**Non-Goals:** - -- Import clangd implementation code directly into `clice` production paths or make the build depend on the vendored reference snapshot. -- Achieve byte-for-byte or range-for-range parity with clangd in this change. -- Add fine-grained folding for every C++ syntax detail such as template parameter lists, requires-clauses, or attribute arguments before their value is proven. -- Introduce editor-specific behavior that only exists to satisfy one frontend. -- Add cross-file or index-backed folding behavior. - -## Decisions - -### 1. Vendor a focused clangd reference snapshot in-tree before implementation work - -The branch should first download a small, reviewable snapshot of clangd's folding-related sources from tag `llvmorg-21.1.8` into `third_party/clangd/llvmorg-21.1.8/`. The snapshot should include the implementation, request plumbing, protocol types, and relevant tests that explain folding behavior, rather than the whole LLVM tree. - -Why: - -- it creates a stable review artifact for this exploration branch -- later implementation commits can point at committed upstream code instead of external URLs -- it keeps the eventual runtime change honest about what is parity work and what is a clice-specific extension - -Alternative considered: - -- Compare against live GitHub URLs only. Rejected because it leaves the analysis outside the repository and makes later review harder. - -### 2. Split the folding-range pipeline into collection, normalization, and rendering - -The current implementation mixes "how a range is discovered" with "how it is emitted as LSP". The new design separates this into three layers: - -- collection: produce internal `RawFoldingRange` entries from AST, comment scanning, and preprocessor metadata -- normalization: sort, deduplicate, validate, and reconcile nested or overlapping ranges -- rendering: decide line/column boundaries, `kind`, and `collapsedText` based on client capabilities - -Why: - -- `lineFoldingOnly`, `collapsedText`, and standards-compatible kind downgrading are rendering concerns and should not pollute collection logic -- comments, macros, and include/import groups do not naturally belong inside the AST visitor -- future range limiting or prioritization should also live in normalization/rendering instead of collector code - -Alternative considered: - -- Keep generating final LSP ranges directly inside the visitor. Rejected because capability negotiation and multi-source collection will keep making the function larger and harder to test. - -### 3. Keep rich internal categories, but only promise standard-compatible public kinds - -Internally, the implementation may still distinguish namespace, class, function body, macro definition, conditional branch, and similar categories so tests, prioritization, and `collapsedText` selection remain precise. However, public LSP output should default to standard kinds only: - -- comment folds -> `comment` -- contiguous include/import groups -> `imports` -- all other structural and preprocessor folds -> `region` - -If some client later proves it needs clice-specific kinds, that can be evaluated separately. This change does not make non-standard kind strings part of the compatibility contract. - -Why: - -- many current custom strings will not be understood by clients and do not produce stable UI semantics -- the real differentiator is what `clice` can fold, not the literal `kind` label -- once public kinds are standardized, `collapsedText` and range boundaries become the primary user-visible expression - -Alternative considered: - -- Continue exposing all custom kinds directly. Rejected because that leaves client compatibility up to luck rather than protocol design. - -### 4. Use the clangd snapshot as a behavior reference, not as a direct implementation template - -clangd's folding logic is text- and token-oriented rather than AST-oriented. `clice` should study the upstream behavior to match the useful parts, but it should not force its own collector architecture to look like clangd's when `CompilationUnitRef::directives()` and the existing AST visitor provide better raw data. - -Why: - -- parity should be measured at the behavior boundary, not by mirroring file structure -- `clice` already has data sources that clangd does not, especially for directive metadata -- this keeps the change focused on correctness and value, not on source-level imitation - -Alternative considered: - -- Rewrite `clice` folding collection to resemble clangd's text parser closely. Rejected because that would discard existing strengths without a clear benefit. - -### 5. Implement comment folding through lexical/source scanning, not AST - -Multiline comments are handled independently in clangd's pseudo-parser path, and `clice` should do the same. The design adds a comment collector that scans the main-file source or token stream directly: - -- fold multiline `/* ... */` block comments -- fold contiguous `//` comment groups -- do not fold single-line comments -- adjust closing boundaries for `lineFoldingOnly` mode so the final visible line is not swallowed incorrectly - -Why: - -- comments are not AST structure, so trying to derive them from AST produces fragile behavior -- lexical scanning naturally handles adjacent-comment grouping and block-comment boundaries - -Alternative considered: - -- Only support block comments. Rejected because clangd already demonstrates that contiguous `//` comment groups are a useful folding case. - -### 6. Rework preprocessor folding around complete branch blocks instead of the current half-open stack - -Today `collect_condition_directives()` only closes the previous branch when it sees `#else`, but when it sees `#endif` it only pops the stack and does not emit a folding range for the final `#if/#elif/#else` branch. As a result, `#if` folding is incomplete. - -The new design treats conditional compilation as an explicit branch-group model: - -- maintain the ordered branch chain for each `#if` group -- allow every branch to close at the next `#elif`, `#else`, or `#endif` -- distinguish active and inactive branches -- allow inactive branches to produce region folds, optionally with distinct `collapsedText` - -Why: - -- this is the minimum sound model needed to fix the current logical gap -- `Condition::ConditionValue` already records true/false/skipped state and can drive inactive-branch folding directly - -Alternative considered: - -- Patch only the `#endif` closing case. Rejected because nested conditions, inactive branches, and range ordering would remain structurally weak. - -### 7. Add dedicated directive-based collectors for macros and include/import groups - -`clice` already collects: - -- `directive.macros` -- `directive.includes` -- `directive.imports` - -The new design therefore adds directive-based folding collectors for: - -- multiline `#define` macro definitions, using continuation backslashes or stable definition ranges -- contiguous `#include` blocks, merged into a single `imports` folding range -- contiguous `import Foo;` / `import Foo:Bar;` module-import blocks, also emitted as `imports` - -Why: - -- the necessary data already exists in preprocessing metadata and does not require new AST modeling -- this is one of the easiest places for `clice` to provide value beyond clangd - -Alternative considered: - -- Leave include/import grouping for a later change. Rejected because the metadata already exists, the implementation cost is relatively low, and the editor-facing value is immediate. - -### 8. Separate clangd parity capabilities from clice-only protocol improvements - -This change should treat `lineFoldingOnly` and standard public kinds as clangd parity work, while `collapsedText` gating and deterministic `rangeLimit` trimming are tracked as clice-side protocol improvements. The implementation can ship them together, but the design and tests should not imply that clangd already provides all of them. - -Why: - -- it keeps the comparison honest -- it allows reviewer discussion to separate "must match upstream baseline" from "valuable extra behavior" -- it keeps spec language compatible with LSP without overstating clangd - -Alternative considered: - -- Treat all capability work as a clangd parity gap. Rejected because clangd's known folding path does not establish that broader claim. - -### 9. Folding-range output must be explicitly bound to client capabilities - -The master server currently only advertises `foldingRangeProvider = true`, but it does not read or propagate folding-specific client capabilities. The new design requires the session to track at least: - -- `lineFoldingOnly` -- whether `collapsedText` is supported -- optional `rangeLimit` - -Rendering rules: - -- when `lineFoldingOnly = true`, only emit ranges that remain meaningful as line-based folds, adjusting end lines where necessary -- when the client does not support `collapsedText`, omit it -- when a `rangeLimit` is declared, trim results deterministically rather than arbitrarily - -Alternative considered: - -- Continue always returning exact columns and `collapsedText`. Rejected because that relies on client tolerance instead of following the protocol contract. - -### 10. Organize tests by source category and protocol behavior - -Tests will be split into two dimensions: - -- source-category unit tests: AST structure, comments, conditional compilation, multiline macros, `#pragma region`, and include/import groups -- protocol-behavior tests: `lineFoldingOnly`, `collapsedText` support, public kind mapping, and range limiting - -In particular, the current `tests/unit/Feature/FoldingRangeTests.cpp` contains `Directive` and `PragmaRegion` cases that do not actually assert results. This change upgrades them into strong assertion-based tests. - -Alternative considered: - -- Rely mostly on manual editor validation. Rejected because folding details regress easily, especially for preprocessor handling and line-only rendering. - -## Risks / Trade-offs - -- [The vendored clangd snapshot could sprawl or become noisy in review] -> Mitigation: vendor only the small folding-related file set needed for comparison and keep it in a dedicated commit. -- [Client capabilities must flow from initialize state into request-time rendering] -> Mitigation: introduce a dedicated folding-options structure so session details do not leak broadly into the feature layer. -- [Inactive-branch and macro-definition ranges can be unstable around expansion locations] -> Mitigation: prefer spelling/main-file ranges and explicitly filter or special-case macro-expansion ranges when necessary. -- [Adding comments, macros, and include/import groups can increase the number of ranges quickly] -> Mitigation: implement stable sorting and `rangeLimit` trimming in the normalization layer. -- [Mapping public kinds back to standard values changes current metadata output] -> Mitigation: the folds themselves remain; the user-visible change is mostly in optional metadata, and tests plus change notes will make that explicit. -- [Multiple collectors may produce overlapping or duplicate ranges] -> Mitigation: normalize by source category and boundary rules so collectors do not amplify noise. - -## Migration Plan - -1. Vendor the focused clangd `llvmorg-21.1.8` folding snapshot into `third_party/clangd/llvmorg-21.1.8/` and commit it separately. -2. Record the confirmed clangd-vs-clice comparison in this change, including which behaviors are parity gaps and which are clice-specific extensions. -3. Refactor the internal folding-range data flow and add render options plus standard kind mapping. -4. Add the comment collector and assertion-backed tests for multiline comment folding. -5. Rewrite conditional-directive and `#pragma region` collection so `#if` branches close correctly through `#endif`. -6. Add multiline macro folding and grouped include/import collectors. -7. Wire folding client capabilities through initialize/request handling and add integration coverage. -8. Add `rangeLimit` trimming and regression cleanup after the new collectors are in place. - -Rollback strategy: - -- If the vendored snapshot becomes more distracting than useful, keep only the documented comparison notes and drop the snapshot commit before merging the implementation branch. -- If protocol negotiation proves unstable, keep the new collectors but temporarily disable outward behavior changes tied to `collapsedText` or `rangeLimit`. -- If a particular new fold category proves noisy, roll it back collector-by-collector instead of reverting the entire folding-range refactor. - -## Open Questions - -- Which exact clangd unit test files from `llvmorg-21.1.8` are worth vendoring alongside `test/folding-range.test`, and which are just noise for this branch? -- Should multiline macro folding cover only the macro body, or the full `#define NAME(...)` line plus body as one fold region? -- Should `rangeLimit` prioritize outer structure, top-of-file regions, or longer ranges when trimming results? -- For structural AST folds originating from macro expansion, should `clice` preserve current behavior or restrict itself to cases with stable spelling ranges only? diff --git a/openspec/changes/improve-folding-range-support/proposal.md b/openspec/changes/improve-folding-range-support/proposal.md deleted file mode 100644 index d8bf8c2a..00000000 --- a/openspec/changes/improve-folding-range-support/proposal.md +++ /dev/null @@ -1,33 +0,0 @@ -## Why - -`clice` already goes beyond clangd in several structural folding cases: it can fold namespaces, records, function parameter lists and bodies, lambda captures, call argument lists, access-specifier sections, and some preprocessor regions. However, the current implementation in `src/Feature/FoldingRange.cpp` still misses several baseline behaviors that clangd already exposes well, especially multiline comment folding, line-only folding rendering, standard public folding kinds, and a stronger regression test matrix. Its preprocessor branch folding is also not yet fully closed. - -This branch is also explicitly about measuring the gap against a fixed upstream reference, not against memory. At tag `llvmorg-21.1.8`, clangd's folding implementation is centered around `clang-tools-extra/clangd/SemanticSelection.cpp`, with request plumbing in `ClangdServer.cpp` and `ClangdLSPServer.cpp`, protocol types in `Protocol.h` and `Protocol.cpp`, and folding coverage in `test/folding-range.test` plus folding-related unit tests. Vendoring that source into the workspace first gives the branch a stable baseline for side-by-side comparison and later commits. - -More importantly, `clice` already has preprocessor metadata that clangd does not fully exploit, such as `directive.macros`, `directive.includes`, `directive.imports`, and evaluated conditional-branch state. That means `clice` should not stop at matching clangd: after filling the real parity gaps, folding ranges can become a more useful C/C++ feature by covering macro definitions, `#if` branches, and include/import groups that clangd does not currently handle well. - -## What Changes - -- Vendor a focused clangd folding-range reference snapshot from tag `llvmorg-21.1.8` into `third_party/clangd/llvmorg-21.1.8/` using `curl` from GitHub raw URLs, then commit that snapshot separately so the comparison baseline is explicit. -- Record a concrete clangd-vs-clice comparison in the tracked change, including the upstream files consulted, confirmed parity gaps, clice-only capabilities, and known implementation bugs. -- Fill the remaining folding-range baseline gaps between `clice` and clangd, especially multiline comment folding, line-only folding rendering, and standard public kind mapping. -- Complete preprocessor-related folding so full `#if/#elif/#else/#endif` branch regions, nested `#pragma region` blocks, and inactive branches have well-defined behavior. -- Add folding features that take advantage of `clice`'s existing preprocessor metadata, including multiline macro definitions and grouped `#include` / `import` blocks. -- Normalize `FoldingRange.kind` output so standard kinds remain compatible while clice-specific fold categories degrade predictably. -- Make folding range responses honor client capabilities such as `lineFoldingOnly`, optional `collapsedText` support, and range limiting. -- Expand unit and integration coverage for AST folds, comments, preprocessor regions, macros, include/import groups, and protocol negotiation behavior. - -## Capabilities - -### New Capabilities -- `folding-ranges`: Provide LSP-compatible, C/C++-focused folding regions that cover AST structure, comments, preprocessor branches, macro definitions, and include/import groups. - -### Modified Capabilities -- None. - -## Impact - -- A vendored comparison snapshot will be added under `third_party/clangd/llvmorg-21.1.8/`, limited to the folding-range implementation, protocol, and test files needed for analysis. -- Primary runtime impact is in `src/Feature/FoldingRange.cpp`, the compile-unit/preprocessor metadata access paths, request handling in `src/Server/Feature.cpp`, and capability negotiation around `src/Server/Lifecycle.cpp`. -- Tests need expansion in `tests/unit/Feature/FoldingRangeTests.cpp`, server/integration coverage, and any required fixtures for preprocessor and module scenarios. -- User-visible behavior will be folding results that are closer to clangd where clangd already has coverage, while also adding high-value C/C++ folds that clangd does not currently provide well, especially macro-definition and conditional-compilation folding. diff --git a/openspec/changes/improve-folding-range-support/specs/folding-ranges/spec.md b/openspec/changes/improve-folding-range-support/specs/folding-ranges/spec.md deleted file mode 100644 index 4087f1f8..00000000 --- a/openspec/changes/improve-folding-range-support/specs/folding-ranges/spec.md +++ /dev/null @@ -1,65 +0,0 @@ -## ADDED Requirements - -### Requirement: Folding range responses honor client capabilities -The server SHALL render folding ranges according to the client's declared folding capabilities instead of always returning the richest possible payload. - -#### Scenario: Line-only folding is respected -- **WHEN** the client declares `textDocument.foldingRange.lineFoldingOnly = true` -- **THEN** the server MUST return folding ranges that remain valid when interpreted as whole-line folds, including adjusting end boundaries for bracketed or comment ranges whose closing delimiter is on the last line - -#### Scenario: Collapsed text is gated by client support -- **WHEN** the client does not declare support for `textDocument.foldingRange.foldingRange.collapsedText` -- **THEN** the server MUST omit `collapsedText` from the folding range response - -#### Scenario: Preferred range limits are applied deterministically -- **WHEN** the client declares `textDocument.foldingRange.rangeLimit = N` and the server can produce more than `N` folding ranges for a document -- **THEN** the server MUST return no more than `N` ranges and MUST choose them using a deterministic ordering rule - -#### Scenario: Standard kinds are emitted compatibly -- **WHEN** a folding range represents a comment block, an include/import block, or any other foldable region -- **THEN** the server MUST emit `kind = comment`, `kind = imports`, or `kind = region` respectively, and MUST NOT require clients to understand clice-specific kind strings in order to fold correctly - -### Requirement: Structural and comment folding baseline -The server SHALL provide folding ranges for multi-line C/C++ structural regions and multi-line comments in the main file. - -#### Scenario: Multi-line comment blocks can be folded -- **WHEN** a document contains a multi-line `/* ... */` comment or a contiguous block of `//` comments spanning more than one line -- **THEN** the server MUST return a folding range for that comment block with `kind = comment` - -#### Scenario: Single-line comments are not folded -- **WHEN** a document contains a single-line comment that does not extend across multiple lines and is not part of a larger contiguous comment block -- **THEN** the server MUST NOT return a folding range for that comment - -#### Scenario: Existing structural regions remain foldable -- **WHEN** a document contains a multi-line namespace, record, function body, parameter list, lambda body, initializer list, or other supported structural region already collected by clice -- **THEN** the server MUST continue to return a folding range for that region if its boundaries can be mapped back to the main file - -### Requirement: Preprocessor regions fold as complete branch blocks -The server SHALL provide complete and nested folding ranges for preprocessor branch structures instead of leaving the final branch in a conditional block unclosed. - -#### Scenario: Final conditional branch closes at endif -- **WHEN** a document contains a `#if/#elif/#else/#endif` chain -- **THEN** the server MUST generate a folding range for each multi-line branch body, including the last branch body that ends at `#endif` - -#### Scenario: Inactive conditional branches can be folded -- **WHEN** a conditional branch is known to be inactive or skipped in the current preprocessing configuration -- **THEN** the server MUST be able to return a folding range covering that inactive branch region using `kind = region` - -#### Scenario: Nested pragma regions are folded -- **WHEN** a document contains nested `#pragma region` / `#pragma endregion` pairs in the main file -- **THEN** the server MUST return properly nested folding ranges for each matched region pair - -### Requirement: C/C++ directive groups and multiline macros are foldable -The server SHALL use clice's preprocessor metadata to expose foldable ranges that clangd does not currently provide. - -#### Scenario: Multi-line macro definitions can be folded -- **WHEN** a document contains a multi-line macro definition whose body spans more than one physical line -- **THEN** the server MUST return a folding range for that macro definition using `kind = region` - -#### Scenario: Consecutive include directives are grouped -- **WHEN** a document contains a contiguous block of `#include` directives with no intervening non-trivia code lines -- **THEN** the server MUST return a folding range covering that include block using `kind = imports` - -#### Scenario: Consecutive module imports are grouped -- **WHEN** a document contains a contiguous block of C++ module `import` declarations with no intervening non-trivia code lines -- **THEN** the server MUST return a folding range covering that import block using `kind = imports` diff --git a/openspec/changes/improve-folding-range-support/tasks.md b/openspec/changes/improve-folding-range-support/tasks.md deleted file mode 100644 index 60c9af3c..00000000 --- a/openspec/changes/improve-folding-range-support/tasks.md +++ /dev/null @@ -1,39 +0,0 @@ -## 1. Reference Snapshot - -- [ ] 1.1 Download the clangd folding-range reference files for `llvmorg-21.1.8` into `third_party/clangd/llvmorg-21.1.8/` using `curl` against GitHub raw URLs. -- [ ] 1.2 Include at least `SemanticSelection.cpp`, `ClangdServer.cpp`, `ClangdLSPServer.cpp`, `Protocol.h`, `Protocol.cpp`, `test/folding-range.test`, and any directly related unit tests needed for comparison. -- [ ] 1.3 Commit the vendored snapshot separately with the upstream tag and file list in the commit message. - -## 2. Comparison and Pipeline - -- [ ] 2.1 Record a side-by-side comparison in the change artifacts between clangd's folding path and clice's current path, calling out confirmed parity gaps, clice-only capabilities, and known bugs. -- [ ] 2.2 Refactor `src/Feature/FoldingRange.cpp` so collection, normalization, and LSP rendering are separated by an internal raw-range model. -- [ ] 2.3 Replace direct exposure of clice-specific public folding kinds with a stable mapping to standard LSP `comment` / `imports` / `region` kinds. - -## 3. Comment and Structural Baseline - -- [ ] 3.1 Add a comment collector that folds multi-line block comments and contiguous multi-line `//` comment groups in the main file. -- [ ] 3.2 Preserve existing AST structural folding behavior while routing it through the new normalization/rendering pipeline. -- [ ] 3.3 Add focused unit tests for comment folding, single-line comment exclusion, and structural folding regressions. - -## 4. Preprocessor Folding - -- [ ] 4.1 Rework conditional-directive collection so each `#if/#elif/#else/#endif` branch body closes correctly, including the final branch ending at `#endif`. -- [ ] 4.2 Add folding support for inactive conditional branches using the existing preprocessor condition metadata. -- [ ] 4.3 Strengthen `#pragma region` handling and convert the current placeholder directive tests into assertion-backed coverage. - -## 5. Protocol and Rendering - -- [ ] 5.1 Capture client folding capabilities during initialize and use them when serving `textDocument/foldingRange`. -- [ ] 5.2 Honor `lineFoldingOnly`, gate `collapsedText`, and apply deterministic `rangeLimit` trimming during folding-range rendering. -- [ ] 5.3 Add integration coverage for line-only rendering, standard kind output, optional collapsed text, and range limiting. - -## 6. Clice-Specific Folding Extensions - -- [ ] 6.1 Add folding ranges for multi-line macro definitions using `directive.macros` and stable main-file source ranges. -- [ ] 6.2 Add grouping folds for contiguous `#include` blocks and return them as `imports` ranges. -- [ ] 6.3 Add grouping folds for contiguous C++ module `import` declarations and cover mixed include/import layouts with tests. - -## 7. Verification - -- [ ] 7.1 Run the relevant folding-range unit and integration tests, then fix any ordering, deduplication, or boundary regressions found during verification. diff --git a/openspec/changes/improve-folding-range-support/.openspec.yaml b/openspec/changes/split-folding-range-pipeline/.openspec.yaml similarity index 50% rename from openspec/changes/improve-folding-range-support/.openspec.yaml rename to openspec/changes/split-folding-range-pipeline/.openspec.yaml index c430c5fa..25345f42 100644 --- a/openspec/changes/improve-folding-range-support/.openspec.yaml +++ b/openspec/changes/split-folding-range-pipeline/.openspec.yaml @@ -1,2 +1,2 @@ schema: spec-driven -created: 2026-04-03 +created: 2026-04-22 diff --git a/openspec/changes/split-folding-range-pipeline/design.md b/openspec/changes/split-folding-range-pipeline/design.md new file mode 100644 index 00000000..4bd03575 --- /dev/null +++ b/openspec/changes/split-folding-range-pipeline/design.md @@ -0,0 +1,104 @@ +## Context + +This change extracts decision `2` from `openspec/changes/explore-improve-folding-range-support/design.md` into a standalone proposal. The current folding implementation in `src/feature/folding_ranges.cpp` mixes three responsibilities in one path: + +- discovering foldable structure from AST data +- deciding which ranges survive deduplication and validation +- shaping the final LSP response, including output metadata + +That coupling makes the code harder to extend safely. Comment folding, directive-based collectors, capability-aware rendering, and range limiting all become riskier when collection and rendering rules share the same code path. The extracted proposal keeps scope narrower: it does not add new fold categories by itself, but it creates the architecture that later changes can build on without destabilizing existing structural folding. + +## Goals / Non-Goals + +**Goals:** + +- Separate folding processing into collection, normalization, and rendering phases. +- Preserve the existing AST structural folding categories already supported by `clice`. +- Make ordering, deduplication, and boundary validation deterministic and testable. +- Give later changes a stable extension point for comments, directives, and client-driven rendering options. + +**Non-Goals:** + +- Add comment folding in this change. +- Fix preprocessor branch-closing behavior in this change. +- Add new fold categories such as macro definitions or include/import grouping. +- Depend on initialize-time client capability plumbing being implemented first. + +## Decisions + +### 1. Introduce a raw internal folding-range model + +Collectors should emit an internal `RawFoldingRange`-style structure instead of final LSP protocol objects. The raw model should preserve source locations, an internal category, and optional metadata hints that later phases may use. + +Why: + +- collectors should describe what was found, not how it will be serialized +- future comment and directive collectors can share the same pipeline contract +- tests can validate collection independently from rendering + +Alternative considered: + +- Continue emitting LSP ranges directly from collectors. Rejected because it keeps protocol concerns entangled with source discovery. + +### 2. Normalize ranges before rendering + +All collected ranges should pass through a normalization step before any response is emitted. Normalization is responsible for deterministic ordering, duplicate removal, and rejection of degenerate or unmappable ranges. + +Why: + +- duplicate or invalid ranges are easier to reason about in one place than across many collectors +- stable ordering reduces regression noise and makes range limiting predictable later +- normalization lets new collectors plug in without each collector re-implementing cleanup logic + +Alternative considered: + +- Let each collector manage its own sorting and duplicate suppression. Rejected because cross-collector interactions would still remain undefined. + +### 3. Keep the current AST visitor as the first collector boundary + +The initial extraction should preserve the current AST visitor as one collector feeding the raw model. This reduces refactor risk while still creating the new phase boundaries. + +Why: + +- the existing structural fold coverage is valuable and should not be rewritten unnecessarily +- an adapter-style refactor is easier to verify against current tests than a full collector redesign + +Alternative considered: + +- Rewrite collection around a brand-new multi-source manager immediately. Rejected because it adds scope before the phase split is proven. + +### 4. Move output shaping into a dedicated renderer + +The renderer should translate normalized ranges into LSP folding ranges. Boundary shaping, output kinds, and optional metadata emission should live there, even if some options still use default values until later protocol plumbing exists. + +Why: + +- rendering rules are a separate concern from source discovery +- later work on line-only output, metadata gating, or public kind mapping should not force collector rewrites +- isolating rendering makes behavioral diffs easier to review + +Alternative considered: + +- Keep final boundary shaping next to the AST collector and only add a small helper for sorting. Rejected because it only moves a symptom, not the architectural problem. + +## Risks / Trade-offs + +- [Refactoring the current path can accidentally change fold ordering] -> Mitigation: add deterministic-order assertions and compare outputs for existing structural fixtures. +- [The raw model could become too abstract too early] -> Mitigation: keep the initial fields minimal and only include data already needed by current structural folds. +- [A renderer abstraction may appear premature before full capability plumbing exists] -> Mitigation: keep default render options aligned with current behavior and treat future options as extension points, not immediate scope. + +## Migration Plan + +1. Introduce raw folding-range and render-option types behind the existing entrypoint. +2. Convert the current AST-based collectors to emit raw ranges. +3. Insert normalization between collection and response emission. +4. Move LSP object construction into a dedicated renderer. +5. Verify that existing structural folding fixtures still produce the expected ranges. + +Rollback strategy: + +- If the refactor destabilizes output, keep the new helper types but temporarily route the old direct-emission path until normalization and rendering regressions are resolved. + +## Open Questions + +- Whether public kind remapping should land in this extracted change or remain a follow-up proposal once the renderer boundary exists. diff --git a/openspec/changes/split-folding-range-pipeline/proposal.md b/openspec/changes/split-folding-range-pipeline/proposal.md new file mode 100644 index 00000000..88d4b94e --- /dev/null +++ b/openspec/changes/split-folding-range-pipeline/proposal.md @@ -0,0 +1,26 @@ +## Why + +`explore-improve-folding-range-support` combines several different concerns: upstream comparison work, baseline folding fixes, preprocessor extensions, and an internal refactor. The second design point in that change, splitting the folding-range pipeline into collection, normalization, and rendering, is the architectural slice that other work depends on and should be referenceable as its own proposal. + +## What Changes + +- Extract the pipeline-splitting work from `explore-improve-folding-range-support` into a standalone change focused on folding-range architecture. +- Introduce an internal raw folding-range model so collectors no longer emit final LSP objects directly. +- Define a normalization phase that performs deterministic sorting, duplicate removal, and boundary validation before response generation. +- Define a rendering phase that owns line/column shaping and optional metadata emission instead of mixing those concerns into collectors. +- Preserve the current AST structural folding coverage while establishing extension points for future comment, directive, and capability-aware rendering work. + +## Capabilities + +### New Capabilities +- `folding-range-pipeline`: Provide a deterministic folding-range pipeline that separates collection, normalization, and rendering while preserving existing structural folds. + +### Modified Capabilities +- None. + +## Impact + +- `src/feature/folding_ranges.cpp` will be refactored around raw-range collection, normalization, and rendering boundaries. +- Folding-related helper types may be introduced near the folding feature implementation. +- `tests/unit/feature/folding_range_tests.cpp` will need regression coverage for structural folds and deterministic ordering. +- `openspec/changes/explore-improve-folding-range-support/design.md` remains the source change from which this standalone proposal was extracted. diff --git a/openspec/changes/split-folding-range-pipeline/specs/folding-range-pipeline/spec.md b/openspec/changes/split-folding-range-pipeline/specs/folding-range-pipeline/spec.md new file mode 100644 index 00000000..3e1eba1e --- /dev/null +++ b/openspec/changes/split-folding-range-pipeline/specs/folding-range-pipeline/spec.md @@ -0,0 +1,38 @@ +## ADDED Requirements + +### Requirement: Folding ranges are normalized before response emission +The server SHALL convert collected folding candidates into a deterministic normalized set before emitting the folding range response. + +#### Scenario: Duplicate candidates collapse to one emitted fold +- **WHEN** multiple collectors produce the same folding candidate for the same source span and internal category +- **THEN** the server MUST emit at most one folding range for that candidate + +#### Scenario: Invalid candidates are dropped during normalization +- **WHEN** a collected folding candidate does not span multiple lines or cannot be mapped back to the main file +- **THEN** the server MUST omit that candidate from the emitted folding ranges + +#### Scenario: Output ordering is deterministic +- **WHEN** the same document is analyzed repeatedly without source changes +- **THEN** the server MUST emit folding ranges in a deterministic order that does not depend on collector traversal order + +### Requirement: Existing structural folding survives the pipeline split +The server SHALL preserve the currently supported AST structural folding categories after collection, normalization, and rendering are separated. + +#### Scenario: Supported structural regions remain foldable +- **WHEN** a document contains a supported multi-line namespace, record, function body, parameter list, lambda body, initializer list, call argument list, or compound statement +- **THEN** the server MUST still return a folding range for that region when its boundaries can be mapped to the main file + +#### Scenario: Structural coverage is preserved through normalization +- **WHEN** the document contains only currently supported AST-driven folding categories +- **THEN** normalization and rendering MUST NOT remove a valid structural fold except when it is an exact duplicate or an invalid range + +### Requirement: Rendering decisions are applied after normalization +The server SHALL derive final LSP folding-range output from normalized internal ranges instead of requiring collectors to emit protocol-shaped results directly. + +#### Scenario: Rendering options do not require collector changes +- **WHEN** rendering rules change how line or metadata output is shaped for a normalized fold +- **THEN** the server MUST apply that change in the rendering phase without requiring collector-specific logic changes + +#### Scenario: Metadata hints remain optional until rendering +- **WHEN** a collected or normalized fold carries optional kind or collapsed-text hints +- **THEN** the renderer MUST decide whether to surface, transform, or suppress that metadata in the emitted LSP range diff --git a/openspec/changes/split-folding-range-pipeline/tasks.md b/openspec/changes/split-folding-range-pipeline/tasks.md new file mode 100644 index 00000000..99bc1ada --- /dev/null +++ b/openspec/changes/split-folding-range-pipeline/tasks.md @@ -0,0 +1,16 @@ +## 1. Raw Model and Collector Boundary + +- [ ] 1.1 Introduce internal raw folding-range and render-option types while keeping the current folding entrypoint stable. +- [ ] 1.2 Convert the existing AST structural folding path in `src/feature/folding_ranges.cpp` to emit raw ranges instead of final LSP ranges. +- [ ] 1.3 Add regression fixtures or assertions that cover the currently supported structural fold categories before further refactoring. + +## 2. Normalization and Rendering + +- [ ] 2.1 Implement normalization for deterministic sorting, duplicate removal, and invalid-range filtering. +- [ ] 2.2 Introduce a dedicated renderer that converts normalized ranges into final LSP folding-range objects. +- [ ] 2.3 Keep default rendered output compatible with current structural behavior while exposing extension points for future collectors and render rules. + +## 3. Verification + +- [ ] 3.1 Compare pre-refactor and post-refactor outputs for the existing structural folding test cases. +- [ ] 3.2 Run relevant folding-range unit tests and fix any ordering, deduplication, or boundary regressions introduced by the new pipeline.