Show enclosing symbol context in code diffs (OD-3091)

Show enclosing symbol context in code diffs #3091

Service Desk opened 1 day ago

Motivation

OneDev diffs show changed lines and expandable unchanged lines, but they do not identify the enclosing class, function, or method. In a long file, reviewers can therefore see what changed without knowing where the change belongs. The problem is more pronounced when the old and new sides correspond to different symbols, such as when code is moved or a method is renamed.

This request follows the finalized design discussion in #3089.

Current UX

┌──────────────────────────────────────────────────────────┐
│ @@ changed lines                                         │
│ - calculateLegacyBalance(account);                       │
│ + calculateCurrentBalance(account);                      │
└──────────────────────────────────────────────────────────┘
                     ↑
       The enclosing method is not visible

Given code such as:

class AccountService {
    reconcileAccounts() {          // important context
        ... many unchanged lines ...
        - calculateLegacyBalance(account);
        + calculateCurrentBalance(account);
    }
}

the reviewer must navigate away from the change or expand surrounding lines to discover that it belongs to AccountService.reconcileAccounts.

Proposed UX

Show both per-hunk context and sticky context. Old and new revision contexts remain independent because they may refer to different symbols.

Split view

┌──────────────────── OLD ────────────────────┬──────────────────── NEW ────────────────────┐
│ Sticky: AccountService.reconcileAccounts    │ Sticky: AccountService.refreshAccounts      │
├─────────────────────────────────────────────┼─────────────────────────────────────────────┤
│ Hunk: AccountService.reconcileAccounts      │ Hunk: AccountService.refreshAccounts        │
│ calculateLegacyBalance(account);            │ calculateCurrentBalance(account);            │
└─────────────────────────────────────────────┴─────────────────────────────────────────────┘

Unified view

┌───────────────────────────────────────────────────────────────────────────────────────────┐
│ OLD: AccountService.reconcileAccounts                                                      │
│ NEW: AccountService.refreshAccounts                                                        │
├───────────────────────────────────────────────────────────────────────────────────────────┤
│ Hunk — OLD: reconcileAccounts | NEW: refreshAccounts                                       │
│ - calculateLegacyBalance(account);                                                         │
│ + calculateCurrentBalance(account);                                                        │
└───────────────────────────────────────────────────────────────────────────────────────────┘

Desired behavior

Display enclosing symbol context in each diff hunk.
Keep the active context visible while scrolling through long hunks.
In split view, render independent sticky elements for the old and new sides.
In unified view, render two compact, clearly labeled sticky elements (OLD and NEW) that update independently.
When old and new contexts differ, show both rather than collapsing them into one label.
When context cannot be determined for one side, leave that side absent or clearly unavailable without hiding the context available for the other side.
Preserve normal diff navigation and line expansion behavior.

Copy

Related
- Question about displaying diff context (OD-3089) Closed
- Show enclosing symbol context in code diffs (OD-3090) Open

Activities

Service Desk commented 1 day ago

@zaakiy, this issue carries forward the finalized design and motivation from #3089, including old-versus-proposed UX diagrams and a pseudocode example. It is assigned to @robin as requested.

Copy
Service Desk referenced from other issue 1 day ago

Question about displaying diff context (OD-3089)

Closed
Service Desk commented 1 day ago

Duplicate of #3090, which was created moments earlier with the same finalized design. Continuing the feature work in #3090.

Copy
Service Desk changed state to 'Closed' 1 day ago
Previous Value Current Value
Open

Closed
Login to comment

Previous Value	Current Value
Open	Closed

Type	New Feature
Priority	Normal
Assignee	Robin Shen
Labels	No labels

Issue Votes (0)

Watchers (3)

Reference

OD-3091