Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-04-15
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
## Context

The `DecoratorManager` class has two independent display toggles:
- `gutterEnabled`: Controls gutter color indicators
- `annotationEnabled`: Controls inline blame annotations

When users disable both features, the `setEditorDecorations()` method correctly clears all decorations. However, the transient decoration flow (triggered during file editing) does not check these flags before applying decorations.

**Current Flow (simplified):**
```
onDidChangeTextDocument
→ scheduleRefresh()
→ applyTransientDecorations() or applyTransientUncommittedDecorations()
→ setEditorDecorations() → clears decorations if both disabled
```

The issue: decorations are computed and applied by the transient methods before `setEditorDecorations()` checks the flags and clears them. This creates a visible flash.

## Goals / Non-Goals

**Goals:**
- Prevent transient decorations from being applied when both display features are disabled
- Maintain existing behavior for all enabled/disabled combinations
- Avoid unnecessary computation when features are disabled

**Non-Goals:**
- No changes to the decoration rendering logic itself
- No changes to configuration or API surface

## Decisions

### Guard Condition Placement

**Decision:** Add the guard condition in `scheduleRefresh()` before calling transient decoration methods.

**Rationale:**
- `scheduleRefresh()` is the entry point for all transient decoration logic
- Early exit avoids unnecessary computation (building transient blame lines, calculating decoration options)
- Single point of control - clear and maintainable

**Alternatives Considered:**
1. **Add guards inside `applyTransientDecorations()` and `applyTransientUncommittedDecorations()`**
- Rejected: Would duplicate the same check in two methods
- Computation would still happen (method parameters evaluated before call)

2. **Check flags in `setEditorDecorations()` and skip clearing**
- Rejected: The core issue is that decorations are applied, then cleared. The flash happens between apply and clear.

### Guard Logic

**Decision:** Skip transient decorations only when BOTH `gutterEnabled` AND `annotationEnabled` are `false`.

**Rationale:**
- If either feature is enabled, transient decorations should be applied
- The existing logic in `setEditorDecorations()` already handles selective display
- This matches the user's mental model: "hide both" means "show nothing"

## Risks / Trade-offs

**Risk:** Edge case where transient decorations are needed for internal state even when not displayed.

**Mitigation:** The transient decorations are only used for display purposes. The persistent `renderedBlameLines` and `renderedLineInfo` maps are maintained separately and updated after the scheduled refresh completes, so internal state remains consistent.

**Trade-off:** Small increase in branching complexity for significant UX improvement.

## Open Questions

None - this is a straightforward bug fix with clear implementation path.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## Why

When users disable inline annotations (`hideBlame`) and gutter decorations (`hideGutter`), they expect to see no blame information whatsoever. However, when editing a file, uncommitted decorations still flash briefly on the screen before being cleared. This creates a jarring user experience and violates the user's intent to disable blame functionality.

## What Changes

- Add guard condition in `scheduleRefresh()` to skip applying transient decorations when both `gutterEnabled` and `annotationEnabled` are `false`
- No changes to public APIs or configuration options
- No breaking changes

## Capabilities

### New Capabilities

None - this is a bug fix within existing behavior.

### Modified Capabilities

None - this fix does not change any documented requirements or specs. It corrects an implementation bug where the enabled state was not being checked before applying transient decorations.

## Impact

- **Affected Code**: `decoratorManager.ts` - `scheduleRefresh()` method
- **Dependencies**: None
- **Performance**: Minor improvement - avoids unnecessary decoration computation when features are disabled
- **Testing**: Should verify that:
1. When blame is fully disabled, editing shows no decorations
2. When only gutter is enabled, editing only shows gutter decorations
3. When only annotations are enabled, editing only shows inline decorations
4. When both are enabled, editing shows both as before
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# No New or Modified Specifications

This change is a bug fix that corrects implementation behavior without changing documented requirements.

## Context

The existing behavior is already specified:
- When users disable blame annotations (via `hideBlame` command), no inline decorations should be displayed
- When users disable gutter decorations (via `hideGutter` command), no gutter indicators should be displayed

This fix addresses an implementation bug where transient decorations were being applied during editing even when both features were disabled. The "flash" of decorations was a violation of existing requirements, not a new requirement.

## Conclusion

No new specs or delta specs are needed. The fix ensures the implementation matches the existing user-facing contract.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## 1. Implementation

- [x] 1.1 Add guard condition in `scheduleRefresh()` method to skip transient decorations when both `gutterEnabled` and `annotationEnabled` are `false`
- [x] 1.2 Verify the guard condition is placed before the `hasStructuralLineChange` and `hasStructuralLineDeletion` checks
- [x] 1.3 Ensure the guard uses short-circuit evaluation: `if (this.gutterEnabled || this.annotationEnabled) { ... }`

## 2. Testing

- [ ] 2.1 Manually test: Disable both blame and gutter, then edit a file - verify no decorations flash
- [ ] 2.2 Manually test: Enable only gutter, edit a file - verify only gutter decorations appear during editing
- [ ] 2.3 Manually test: Enable only annotations, edit a file - verify only inline decorations appear during editing
- [ ] 2.4 Manually test: Enable both, edit a file - verify both decorations appear as before

## 3. Verification

- [ ] 3.1 Run existing test suite to ensure no regressions
- [ ] 3.2 Check that `setEditorDecorations()` logic remains unchanged (still handles selective display correctly)
- [ ] 3.3 Verify no console errors or warnings when editing with features disabled
11 changes: 7 additions & 4 deletions src/decoratorManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,13 @@ export class DecoratorManager implements vscode.Disposable {
this.blameManager.invalidateDocument(document);
const hasLineDeletion = hasStructuralLineDeletion(contentChanges);

if (!hasLineDeletion && hasStructuralLineChange(contentChanges)) {
this.applyTransientDecorations(document, contentChanges);
} else if (hasLineDeletion) {
this.applyTransientUncommittedDecorations(document, contentChanges);
// Only apply transient decorations when at least one display feature is enabled
if (this.gutterEnabled || this.annotationEnabled) {
if (!hasLineDeletion && hasStructuralLineChange(contentChanges)) {
this.applyTransientDecorations(document, contentChanges);
} else if (hasLineDeletion) {
this.applyTransientUncommittedDecorations(document, contentChanges);
}
}

const pending = this.pendingRefreshes.get(documentKey);
Expand Down