Skip to content

Commit 0d83878

Browse files
committed
[adr] Add 0020, clean up adr-process, remove OLO-renaming from 0019
1 parent befa93f commit 0d83878

3 files changed

Lines changed: 99 additions & 70 deletions

File tree

doc/adr-process.md

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Each ADR has a status indicating its current state:
3838

3939
| Status | Meaning | Usage |
4040
|--------|---------|-------|
41+
| **Draft** | Exploratory, capturing ideas and direction | Early thinking, not yet a concrete proposal |
4142
| **Proposed** | Under discussion, not yet decided | Decision documented, seeking feedback/approval |
4243
| **Accepted** | Decision approved, implementation in progress or planned | Commitment made but work not complete |
4344
| **Implemented** | Fully completed and deployed | Decision executed, feature/change live in codebase |
@@ -84,46 +85,6 @@ git add adr/0015-my-decision.md
8485
git commit -m "[adr] Add ADR-0015: My Decision"
8586
```
8687

87-
## ADR Index
88-
89-
### Active Decisions
90-
91-
- [ADR-0001](0001-store-reference-data-in-text-files-with-encoding.md) - Store reference data in text files with encoding
92-
- [ADR-0002](0002-remove-enum-and-struct-fill-macro-generation.md) - Remove enum and struct fill macro generation
93-
- [ADR-0003](0003-use-markdown-architectural-decision-records.md) - Use Markdown architectural decision records
94-
- [ADR-0004](0004-remove-html-generation.md) - Remove HTML generation
95-
- [ADR-0005](0005-automatically-find-config-files.md) - Automatically find config files
96-
- [ADR-0006](0006-remove-prechecks.md) - Remove prechecks
97-
- [ADR-0007](0007-use-classic-adr-s-for-architecture-decisions.md) - Use classic ADRs for architecture decisions
98-
- [ADR-0008](0008-remove-no-stdoptions.md) - Remove no-stdoptions
99-
- [ADR-0009](0009-remove-alignment.md) - Remove alignment *(Implemented)*
100-
- [ADR-0010](0010-remove-support-for-javadoc.md) - Remove support for Javadoc
101-
- [ADR-0011](0011-remove-support-for-java.md) - Remove support for Java
102-
- [ADR-0012](0012-remove-lexem-stream-caching.md) - Remove lexem stream caching mechanism *(Implemented)*
103-
- [ADR-0013](0013-limited-extern-detection-in-c-files.md) - Limited detection of archaic extern declarations in C source files *(Proposed)*
104-
- [ADR-0014](0014-adopt-on-demand-parsing-architecture.md) - Adopt on-demand parsing architecture *(Proposed)*
105-
106-
### By Status
107-
108-
**Implemented:**
109-
- [ADR-0009](0009-remove-alignment.md) - Remove alignment
110-
- [ADR-0012](0012-remove-lexem-stream-caching.md) - Remove lexem stream caching
111-
112-
**Proposed:**
113-
- [ADR-0013](0013-limited-extern-detection-in-c-files.md) - Extern detection limitations
114-
- [ADR-0014](0014-adopt-on-demand-parsing-architecture.md) - On-demand parsing
115-
116-
**Accepted:** (implementation ongoing or planned)
117-
- [ADR-0001](0001-store-reference-data-in-text-files-with-encoding.md) through [ADR-0011](0011-remove-support-for-java.md)
118-
119-
### By Topic
120-
121-
**Feature Removal / Simplification:**
122-
- ADR-0002, 0004, 0006, 0008, 0009, 0010, 0011, 0012
123-
124-
**Architecture & Design:**
125-
- ADR-0001, 0003, 0005, 0007, 0013, 0014
126-
12788
## ADR References
12889

12990
- [Michael Nygard's original blog post](https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions) introducing ADRs

doc/adr/0019-unify-browse-terminology.md

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ Proposed
1111
"Goto" is currently used ambiguously at multiple levels:
1212

1313
- User-facing commands (menus, Emacs functions, documentation)
14-
- Server operations (OLO_* enum values)
14+
- Server operations (OP_* enum values)
1515
- Protocol commands (ppcGotoPosition)
16+
- Internal function names (e.g., functions using "goto" to mean "issue positional response" or "navigate to")
1617

1718
This creates confusion as "goto" means different things: user intent to explore code, server computation of which reference, and low-level cursor positioning.
1819

@@ -22,60 +23,40 @@ Additionally, "Browsing" as a concept already exists in documentation and the ex
2223

2324
Adopt "Browse" as the umbrella term for reference navigation. Unify user command names with server operation names.
2425

25-
**Entry points into browsing:**
26-
27-
| Current | Proposed |
28-
|---------|----------|
29-
| `OLO_PUSH` | `BROWSE_PUSH` or `PUSH_AND_BROWSE` |
30-
| `OLO_COMPLETION_GOTO` | `COMPLETION_BROWSE` |
31-
| `OLO_TAG_SEARCH` | `SEARCH_BROWSE` |
32-
33-
**Navigation within browsing (shared across all contexts):**
34-
35-
| Current | Proposed |
36-
|---------|----------|
37-
| `OLO_NEXT` | `BROWSE_NEXT_REFERENCE` |
38-
| `OLO_PREVIOUS` | `BROWSE_PREVIOUS_REFERENCE` |
39-
| `OLO_GOTO_DEF` | `BROWSE_GOTO_DEFINITION` |
40-
| `OLO_POP` | `BROWSE_POP` |
41-
| `OLO_REF_FILTER_SET` | `BROWSE_SET_FILTER` |
42-
43-
**Protocol layer (unchanged):**
44-
45-
- `ppcGotoPosition` or rename to `ppcPositionCursor` - low-level editor instruction
46-
4726
**Naming pattern:**
4827
- `BROWSE_*` = navigation within browsing mode
4928
- `*_BROWSE` = entry point that starts browsing
5029

51-
=======
30+
**Protocol layer:**
31+
32+
- `ppcGotoPosition` or rename to `ppcPositionCursor` - low-level editor instruction, distinct from user-level "browse"
33+
5234
## Architectural Insight
5335

5436
Browsing, Completion, and Search share the same navigation sub-operations but operate on different stacks:
5537

5638
| Domain | Stack | Entry Point | Navigation |
5739
|--------|-------|-------------|------------|
5840
| Browsing | browsingStack | PUSH | NEXT, PREVIOUS, POP, FILTER |
59-
| Completion | completionStack | COMPLETION | FORWARD, BACK, GOTO |
60-
| Search | searchingStack | TAG_SEARCH | FORWARD, BACK, GOTO |
41+
| Completion | completionStack | COMPLETION | NEXT, PREVIOUS, GOTO_N |
42+
| Search | searchingStack | SEARCH | NEXT, PREVIOUS, GOTO_N |
6143

6244
This suggests a common "exploration mode" abstraction with domain-specific entry points but shared navigation semantics. The terminology unification should reflect this pattern - each domain has its entry operation, then uses common navigation verbs.
6345

64-
## Consequences
46+
## Consequences and Risks
6547

6648
**Benefits:**
6749

6850
- User command name = server operation name (one concept instead of two)
6951
- "Browse" clearly indicates "exploration mode with back-stack"
7052
- Distinguishes user intent from low-level cursor positioning
7153
- Consistent with existing "Browser" terminology in docs
72-
- Completion, search, and push-based browsing share the same sub-commands
7354

7455
**Risks:**
7556

76-
- Renaming OLO_* values affects multiple files
7757
- Emacs function names may need updating for consistency
58+
- Internal function names using "goto" ambiguously need careful review to determine actual intent
7859

79-
## Alternatives Considered
60+
## Considered Options
8061

8162
TBD
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# Separate buffer synchronization from operation dispatch
2+
3+
Date: 2026-02-11
4+
5+
## Status
6+
7+
Draft
8+
9+
## Deciders
10+
11+
Thomas Nilefalk, Claude (AI pair programmer)
12+
13+
## Problem Statement and Context
14+
15+
The current server architecture interleaves buffer freshness checking with operation execution. When the editor client sends a request (e.g., BROWSE_NEXT), the operation handler discovers mid-execution that a file is stale, triggers a re-parse, then continues with refreshed data. This design has been a persistent source of bugs, particularly with the "auto-updating" browser stack where pointers become dangling after mid-operation refreshes.
16+
17+
The current flow:
18+
19+
```
20+
Client request (with preloaded files for current buffer)
21+
-> determine operation
22+
-> during operation, discover staleness
23+
-> re-parse mid-operation
24+
-> continue operation with refreshed data
25+
```
26+
27+
Additionally, the Emacs client currently only preloads the current buffer, meaning the server may have stale data for other modified-but-unsaved files.
28+
29+
### Relation to internal operations
30+
31+
This issue is compounded by the internal operation mechanism where `refactory.c` re-enters the server via `parseBufferUsingServer()` with command-line flags. These internal re-entries also face staleness issues. Separating sync from dispatch would simplify both external and internal operation handling.
32+
33+
## Decision Outcome
34+
35+
*To be determined.* The direction being explored is:
36+
37+
1. **Client sends all modified buffers** with every request, not just the current one. Buffers unchanged since the last request would still be sent but skipped by the server's existing timestamp-based freshness check (the cost is writing to temp files, not re-parsing).
38+
39+
2. **Server refreshes on entry**, before operation dispatch. A dedicated "sync phase" at the server entry point updates the in-memory reference database from all preloaded files.
40+
41+
3. **Operations assume fresh data.** Individual operation handlers (BROWSE_NEXT, etc.) no longer need staleness checks or mid-operation re-parsing.
42+
43+
Proposed flow:
44+
45+
```
46+
Client request (with ALL modified buffers)
47+
-> sync phase: update reference database from modified files
48+
-> perform operation on guaranteed-fresh data
49+
```
50+
51+
### Migration path
52+
53+
Each step is independently valuable and testable:
54+
55+
1. Fix Emacs client to always send all modified buffers (not just the current one)
56+
2. Hoist the "refresh from preloads" phase to server entry, before operation dispatch
57+
3. Remove staleness checks from individual operations
58+
4. The LSP server gets the same "update phase then operate" structure naturally
59+
60+
### Alignment with LSP
61+
62+
This design aligns with LSP's model where `textDocument/didChange` notifications are separate from operation requests. The sync phase is conceptually equivalent to processing accumulated `didChange` events before handling the actual request.
63+
64+
## Consequences and Risks
65+
66+
**Benefits:**
67+
68+
- Eliminates the class of bugs caused by mid-operation refresh (dangling pointers, inconsistent state)
69+
- Operation code becomes simpler - can trust its data
70+
- Easier to test: sync and operations are independently testable
71+
- Natural fit for LSP server architecture
72+
- Internal operations (refactory.c re-entries) also benefit from guaranteed-fresh data
73+
74+
**Risks:**
75+
76+
- Performance: refreshing all modified files on every request may do more work than lazy per-file refresh. In practice, few files change between requests during normal editing.
77+
- Radical rename scenario (many files modified) could be costly, but users typically save and test frequently during such operations.
78+
- Requires changes to both client (Emacs) and server.
79+
80+
**Open questions:**
81+
82+
- What is the actual cost of the client writing all modified buffers to temporary files on every request?
83+
- Could checksums or sequence numbers optimize the "send all modified buffers" step if disk I/O ever becomes a bottleneck?
84+
85+
## Considered Options
86+
87+
*To be explored further as this ADR matures from Draft to Proposed.*

0 commit comments

Comments
 (0)