Skip to content

Commit ec7edf9

Browse files
Put the error actions on the title line and collapse hints
Show in state machine sits on the line of the error title with the other actions, and hints start closed. A hint the user opens stays open after a new verification. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 523d7c3 commit ec7edf9

8 files changed

Lines changed: 106 additions & 33 deletions

File tree

‎client/src/webview/diagnostic-reveal.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,12 @@ export function getDiagnosticRevealTargetFromKey(value?: string | null): Diagnos
2929
}
3030
};
3131
}
32+
33+
/**
34+
* Gets a key that identifies a diagnostic across verifications
35+
* This is its reveal key, or, without a position, its file, title, and message, which never match the reveal key pattern
36+
*/
37+
export function getDiagnosticKey(diagnostic: LJDiagnostic): string {
38+
const target = getDiagnosticRevealTarget(diagnostic);
39+
return target ? getDiagnosticRevealTargetKey(target) : `${diagnostic.file}#${diagnostic.title}#${diagnostic.message}`;
40+
}

‎client/src/webview/script.spec.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,29 @@ describe('webview script', () => {
402402
showDiagnostics([stateError('State refinement error', errorMachine('Counter', 'close'))]);
403403

404404
expect(root.querySelector('.diagnostic-state-machine-btn')?.textContent?.trim()).toBe('Show in state machine');
405-
expect(root.querySelector('.diagnostic-actions .diagnostic-state-machine-btn')).toBeNull();
406-
// right below the error message, before its details
407-
const firstSection = root.querySelector('.error-item .section')!;
408-
expect(root.querySelector('.diagnostic-state-machine-btn')!.compareDocumentPosition(firstSection)).toBe(Node.DOCUMENT_POSITION_FOLLOWING);
405+
// on the line of the title, before the other actions
406+
const actions = root.querySelector('.error-item .diagnostic-title-row .diagnostic-actions')!;
407+
expect(actions.firstElementChild?.classList.contains('diagnostic-state-machine-btn')).toBe(true);
408+
expect(actions.querySelector('.copy-diagnostic-btn')).not.toBeNull();
409+
});
410+
411+
it('keeps hints closed until opened, and open after a new verification', () => {
412+
const error = { ...stateError('State refinement error', errorMachine('Counter', 'close')), hint: 'Call open() first' };
413+
showDiagnostics([error]);
414+
const hint = () => root.querySelector<HTMLDetailsElement>('.error-item .hint-section')!;
415+
expect(hint().open).toBe(false);
416+
expect(hint().querySelector('summary')?.textContent).toBe('Hint');
417+
expect(hint().querySelector('pre')?.textContent).toBe('Call open() first');
418+
419+
hint().open = true;
420+
hint().dispatchEvent(new Event('toggle'));
421+
receive({ type: 'diagnostics', diagnostics: [{ ...error }] });
422+
expect(hint().open).toBe(true);
423+
424+
hint().open = false;
425+
hint().dispatchEvent(new Event('toggle'));
426+
receive({ type: 'diagnostics', diagnostics: [{ ...error }] });
427+
expect(hint().open).toBe(false);
409428
});
410429

411430
it('opens on the first state error of a newly opened file', () => {

‎client/src/webview/script.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type { LJDiagnostic, RefinementMismatchError, StateRefinementError } from
77
import type { Range } from "../types/context";
88
import type { LJStateMachine } from "../types/fsm";
99
import type { ExtensionStatus } from "../state";
10-
import type { NavTab } from "./views/sections";
10+
import { setHintOpen, type NavTab } from "./views/sections";
1111
import { copyDiagnosticToClipboard, getDisplayDiagnostics, renderDiagnosticsView } from "./views/diagnostics/diagnostics";
1212
import type { LJContext } from "../types/context";
1313
import { ContextSectionState, renderContextView } from "./views/context/context";
@@ -56,6 +56,13 @@ export function getScript(vscode: VSCodeApi, document: Document, window: Window)
5656
vscode.postMessage({ type: 'ready' });
5757

5858
// on click
59+
// toggle does not bubble, so it is caught on its way down
60+
root.addEventListener('toggle', (e: Event) => {
61+
const details = e.target as HTMLDetailsElement;
62+
const key = details.getAttribute?.('data-hint-key');
63+
if (key !== null && key !== undefined) setHintOpen(key, details.open);
64+
}, true);
65+
5966
root.addEventListener('click', (e: MouseEvent) => {
6067
const target = e.target instanceof Element ? e.target : null;
6168
if (!target) return;

‎client/src/webview/state-errors.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { LJDiagnostic, StateRefinementError } from "../types/diagnostics";
2-
import { getDiagnosticRevealTarget, getDiagnosticRevealTargetKey } from "./diagnostic-reveal";
2+
import { getDiagnosticKey, getDiagnosticRevealTargetKey } from "./diagnostic-reveal";
33

44
/**
55
* Gets the state errors reported in the current file, in source order
@@ -28,9 +28,7 @@ export function getAllStateErrors(diagnostics: LJDiagnostic[] | undefined): Stat
2828
}
2929

3030
export function getStateErrorKey(error: StateRefinementError): string {
31-
const target = getDiagnosticRevealTarget(error);
32-
// errors without a position never match the position pattern of reveal keys
33-
return target ? getDiagnosticRevealTargetKey(target) : `${error.file}#${error.title}#${error.message}`;
31+
return getDiagnosticKey(error);
3432
}
3533

3634
/**

‎client/src/webview/styles.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,12 +195,28 @@ export function getStyles(): string {
195195
line-height: 1;
196196
pointer-events: none;
197197
}
198+
.diagnostic-title-row {
199+
display: flex;
200+
flex-wrap: wrap;
201+
align-items: center;
202+
justify-content: space-between;
203+
gap: 0.25rem 0.75rem;
204+
}
205+
.diagnostic-title-row h3 {
206+
min-width: 0;
207+
}
198208
.diagnostic-actions {
199-
position: absolute;
200-
top: 0.5rem;
201-
right: 0.5rem;
202209
display: flex;
210+
align-items: center;
203211
gap: 0.25rem;
212+
margin-left: auto;
213+
}
214+
.hint-summary {
215+
font-weight: bold;
216+
cursor: pointer;
217+
}
218+
.hint-section[open] .hint-summary {
219+
margin-bottom: 0.25rem;
204220
}
205221
.copy-diagnostic-btn,
206222
.diagnostic-context-btn {
@@ -252,8 +268,7 @@ export function getStyles(): string {
252268
display: inline-flex;
253269
align-items: center;
254270
gap: 0.4rem;
255-
margin: 0 0 1rem;
256-
padding: 0.25rem 0.6rem;
271+
padding: 0.15rem 0.5rem;
257272
color: var(--vscode-button-secondaryForeground);
258273
background-color: var(--vscode-button-secondaryBackground);
259274
font-size: 0.85rem;

‎client/src/webview/views/diagnostics/errors.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,19 @@ import type {
1414
SyntaxError,
1515
} from "../../../types/diagnostics";
1616
import { renderCopyDiagnosticButton } from "./diagnostics";
17+
import { getDiagnosticKey } from "../../diagnostic-reveal";
1718

1819
export function renderErrors(errors: LJError[]): string {
1920
return /*html*/`
2021
<ul>
2122
${errors.map((error, index) => {
2223
return /*html*/`
2324
<li class="diagnostic-item error-item" ${renderDiagnosticDataAttributes(error)}>
24-
<div class="diagnostic-actions">
25+
${renderError(error, /*html*/`
26+
${renderDiagnosticStateMachineAction(error, index)}
2527
${renderDiagnosticContextAction(error)}
2628
${renderCopyDiagnosticButton('error', index)}
27-
</div>
28-
${renderError(error, renderDiagnosticStateMachineAction(error, index))}
29+
`)}
2930
</li>
3031
`;
3132
}).join("")}
@@ -69,15 +70,15 @@ const errorContentRenderers: ErrorRendererMap = {
6970
/**
7071
* Renders the error content
7172
* @param error The error to render
72-
* @param headerActions Actions shown below the error message
73+
* @param actions Actions shown on the line of the error title
7374
*/
74-
export function renderError(error: LJError, headerActions = ''): string {
75+
export function renderError(error: LJError, actions = ''): string {
7576
const message = error.type === 'refinement-error' || error.type === 'state-refinement-error' ? error.customMessage : error.message;
76-
const header = renderDiagnosticHeader(error.title, message || '');
77+
const header = renderDiagnosticHeader(error.title, message || '', actions);
7778
const content = (errorContentRenderers[error.type] as (error: LJError) => string)?.(error) || '';
78-
const hint = renderHint(error.hint);
79+
const hint = renderHint(error.hint, getDiagnosticKey(error));
7980
const location = renderLocation(error);
80-
return /*html*/`${header}${headerActions}${content}${hint}${location}`;
81+
return /*html*/`${header}${content}${hint}${location}`;
8182
}
8283

8384
function renderDiagnosticContextAction(error: LJError): string {

‎client/src/webview/views/diagnostics/warnings.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
import type { CustomWarning, ExternalClassNotFoundWarning, ExternalMethodNotFoundWarning, LJWarning, UnsatisfiableRefinementWarning } from "../../../types/diagnostics";
22
import { renderDiagnosticDataAttributes, renderExpressionSection, renderDiagnosticHeader, renderHint, renderLocation } from "../sections";
33
import { renderCopyDiagnosticButton } from "./diagnostics";
4+
import { getDiagnosticKey } from "../../diagnostic-reveal";
45

56
export function renderWarnings(warnings: LJWarning[]): string {
67
return /*html*/`
78
<ul>
89
${warnings.map((warning, index) => /*html*/`
910
<li class="diagnostic-item warning-item" ${renderDiagnosticDataAttributes(warning)}>
10-
<div class="diagnostic-actions">
11-
${renderCopyDiagnosticButton('warning', index)}
12-
</div>
13-
${renderWarning(warning)}
11+
${renderWarning(warning, renderCopyDiagnosticButton('warning', index))}
1412
</li>
1513
`).join("")}
1614
</ul>
@@ -33,10 +31,10 @@ const warningContentRenderers: WarningContentRenderers = {
3331
"custom-warning": (_: CustomWarning) => ""
3432
};
3533

36-
export function renderWarning(warning: LJWarning): string {
37-
const header = renderDiagnosticHeader(warning.title, warning.message);
34+
export function renderWarning(warning: LJWarning, actions = ''): string {
35+
const header = renderDiagnosticHeader(warning.title, warning.message, actions);
3836
const content = (warningContentRenderers[warning.type] as (warning: LJWarning) => string)?.(warning) || '';
39-
const hint = renderHint(warning.hint);
37+
const hint = renderHint(warning.hint, getDiagnosticKey(warning));
4038
const location = renderLocation(warning);
4139
return /*html*/`${header}${content}${hint}${location}`;
4240
}

‎client/src/webview/views/sections.ts‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,29 @@ export const renderSection = (title: string, body: string): string => /*html*/
2121
export const renderExpressionSection = (title: string, expression: string): string => /*html*/
2222
renderCustomSection(title, renderHighlightedExpression(expression));
2323

24-
export const renderHint = (hint: string | null): string =>
25-
hint?.trim() ? renderCustomSection('Hint', `<pre>${escapeHtml(hint.trim())}</pre>`) : '';
24+
// hints the user opened, kept open when the diagnostics are rendered again
25+
const openHints = new Set<string>();
26+
27+
export function setHintOpen(key: string, open: boolean) {
28+
if (open) openHints.add(key);
29+
else openHints.delete(key);
30+
}
31+
32+
/**
33+
* Renders the hint of a diagnostic, closed until the user opens it
34+
* @param hint The hint text
35+
* @param key Identifies the diagnostic, so an opened hint stays open after a new verification
36+
*/
37+
export function renderHint(hint: string | null, key?: string): string {
38+
if (!hint?.trim()) return '';
39+
const open = key !== undefined && openHints.has(key);
40+
return /*html*/`
41+
<details class="section hint-section"${key !== undefined ? ` data-hint-key="${escapeHtml(key)}"` : ''}${open ? ' open' : ''}>
42+
<summary class="hint-summary">Hint</summary>
43+
<pre>${escapeHtml(hint.trim())}</pre>
44+
</details>
45+
`;
46+
}
2647

2748
export const renderToggleSection = (title: string, targetId: string, isExpanded: boolean = true): string => /*html*/`
2849
<button class="context-toggle-btn" data-context-toggle="${targetId}" aria-expanded="${isExpanded ? 'true' : 'false'}" type="button">
@@ -31,8 +52,13 @@ export const renderToggleSection = (title: string, targetId: string, isExpanded:
3152
</button>
3253
`;
3354

34-
export const renderDiagnosticHeader = (title: string, message: string): string => /*html*/
35-
`<h3>${title}</h3><div class="diagnostic-header"><p>${message}</p></div>`;
55+
export const renderDiagnosticHeader = (title: string, message: string, actions = ''): string => /*html*/`
56+
<div class="diagnostic-title-row">
57+
<h3>${title}</h3>
58+
${actions.trim() ? `<div class="diagnostic-actions">${actions}</div>` : ''}
59+
</div>
60+
<div class="diagnostic-header"><p>${message}</p></div>
61+
`;
3662

3763
export function renderDiagnosticDataAttributes(diagnostic: LJDiagnostic): string {
3864
const target = getDiagnosticRevealTarget(diagnostic);

0 commit comments

Comments
 (0)