Repository navigation
Wrap the Verification panel at spaces and operators, one conjunct per line - #189
Conversation
Clean-code reviewI audited this diff with the clean-code-principles checklist. Fixed
Declined
|
Codex review, round 1
Fixed (both)
Tests: 127 unit, typecheck, lint, and 39 server tests pass. |
Codex review, round 2
Fixed. A changed predicate is now split into lines on both sides. The old and new lines are paired with the same Tests: 127 unit, typecheck, lint, and 39 server tests pass. |
Codex review, round 3
The loop ends here: nothing to fix. |
… line Expected and every Found line now show one top-level conjunct per line, and a conditional's condition and branches each on a line of their own, with a hanging indent for wrapped lines. The VC cells no longer break anywhere, so a name is split only when it alone is wider than the line. A narrow panel puts each binder above its predicate, with tighter padding and line height. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erals Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
600f884 to
79c1800
Compare
Closes #188 ## What changes The editor diagnostic that `LJDiagnosticsHandler` publishes now carries: 1. the headline (`<title>: <message>`), as before; 2. then each of the verifier's **message notes**, `LJDiagnostic.getMessageNotes()`, one per line; 3. then the verifier's **counterexample**, `getCounterexampleStr()`, when it has one. That text is what the hover and the Problems view show. **Hints are left out**, since they stay collapsed in the panel by design. The server reads the verifier's generic `LJDiagnostic` API, not a per-error list. So any note the verifier adds later appears in the hover with no extension change. That includes the planned From note (liquid-java/liquidjava#522), and a counterexample for state errors once the verifier gives one. ## Before → after (hover text, verifier 0.0.37) **A2_04** ```diff Refinement Error: expected timeout >= 0 but could not prove it + Counterexample: timeout == -1 ``` **A2_10** ```diff Refinement Error: failures.size() == size(this.failures) && failures.size() >= 0 is not a subtype of failures.size() > 0 + Counterexample: failures.size() == 0 ``` A1_38, A1_01, A1_56 and A1_09 are unchanged. Their errors have no message notes and no counterexample in 0.0.37 or main. | | Before | After | |---|---|---| | A2_04 | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/190/A2_04-before.png" width="440"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/190/A2_04-after.png" width="440"> | | A2_10 | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/190/A2_10-before.png" width="440"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/190/A2_10-after.png" width="440"> | (Headless VS Code, 1600×900, Dark Modern.) ## Verifier version **0.0.37**, already released. `getMessageNotes()` and `getCounterexampleStr()` are both in it, and the branch builds with the current pom. Today the only message notes are the `where <alias> means …` lines of refinement errors that use aliases, and no study task has one. The From note will come with a later verifier release. ## Open question for the PM The issue lists the `where … means …` gloss among the notes. In the verifier, the gloss is a **declaration note** (`getDeclarationNotes()`, liquid-java/liquidjava#517, needs 0.0.38). It explains the "declared here" excerpt, which the hover does not show. So this PR shows only the message notes, as specified. If the hover should also show the gloss, that is one more `lines.addAll(diagnostic.getDeclarationNotes())`, but it needs the 0.0.38 bump that #191 makes. ## Overlap with open PRs Server only, in `LJDiagnosticsHandler`. No conflict with #185, #186, #189 or #191. ## Checks Server `mvn -B test` (40 passed; new `LJDiagnosticsHandlerTest`, which fails on main), `npm run test:unit` (124 passed), `npm run test:typecheck`, `npm run lint`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Closes #181 ## What changes - **Server.** `RefinementErrorDTO` and `StateRefinementErrorDTO` now send: - `declarationPositions`, from `LJDiagnostic.getDeclarationPositions()`; - `declarationNotes`, the `where … means …` lines, from `getDeclarationNotes()`. Both are read through the verifier's generic `LJDiagnostic` API. So when the verifier adds notes or positions, they appear with no extension change. `declarationPosition`, the first position, stays as it is for the State Machine view. - **Client.** The Locations section links to **every** declaration position, then shows the notes under the links, muted and with a hanging indent. A declaration in value form (`doorState(this) == closed`), or one with unnamed sets, has no notes, so nothing changes for it. - **Fallback to one position.** The verifier's default `getDeclarationPositions()` returns the single `getDeclarationPosition()`. Refinement errors and any other error with one declaration therefore show one link, as before. ## Verifier version: needs 0.0.38 (not yet released) | Part | Verifier change | |---|---| | `declarationNotes` (the gloss) | liquid-java/liquidjava#517 (merged in main) | | `declarationPositions` (every `from`, e.g. A1_01 lines 110 and 111) | liquid-java/liquidjava#525 (**open**) | The pom is bumped to `0.0.38`, so **CI's server build fails until 0.0.38 is on Maven Central**. Please merge this PR only after a verifier release that has both #517 and #525. If #525 is not merged, drop the `declarationPositions` lines from the two DTOs (one line each) and use `declarationPosition` in `renderLocation` again. The gloss part only needs #517. I tested it locally against #525's head (`9d1bb1e`, which contains main), installed as 0.0.38 in an isolated Maven repository. ## Before → after (Locations section, at the study width) Before is main, with its server built on verifier main. After is this PR on #525's head. Headless VS Code, 1600×900, Dark Modern, default side bar. **A1_01**: both `from`s, each with its gloss ```diff E_OSS_A1_01_SlideRecoder.java:136 ImageWriteParamRefinements.java:110 + ImageWriteParamRefinements.java:111 + where compressionExplicit(this) means compression(this) == compressionExplicit + where compressionTypeSet(this) means compression(this) == compressionTypeSet ``` **A1_38** ```diff E_OSS_A1_38_FormatReaderManager.java:157 BufferedInputStreamRefinements.java:81 + where marked(this) means markState(this) == marked, open(this) means stream(this) == open ``` **A1_56**: `+ where redoable(this) means redoState(this) == redoable` **A1_09**: `+ where notDisplayable(this) means display(this) == notDisplayable` **A2_04**: unchanged. `@Refinement("_ >= 0")` has no gloss. The missing Hint in the after shot comes from #525, not from this PR. | A1_01 | A1_38 | |---|---| | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/191/A1_01.png" width="420"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/191/A1_38.png" width="420"> | | A1_56 | A1_09 | |---|---| | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/191/A1_56.png" width="420"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/191/A1_09.png" width="420"> | <details><summary>A2_04</summary> <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/191/A2_04.png" width="420"> </details> The A1_01 and A1_09 shots are scrolled to Locations. The yellow outline is the focus left on Hints by the screenshot script. ## Overlap with open PRs - **#186 (Context tab)** may conflict in `client/src/webview/views/sections.ts`. It changes `renderVariableHighlightButton` and the imports, right after `renderLocation`, which this PR edits. - **#185 (Found as facts)** may conflict in `client/src/types/diagnostics.ts`. Both PRs add fields to `RefinementError` and `StateRefinementError`, in nearby hunks. No conflict in logic. - **#189 (panel wrapping)**: the notes are in the Locations `<pre>`, which keeps `break-word`, so a long `compressionExplicit(this)` can still wrap inside the name at 240 px (see A1_01). They do not share files apart from `styles.ts`, in separate rules. ## Checks `npm run test:unit` (125 passed; the new test fails on main and also checks the copied text), `npm run test:typecheck`, `npm run lint`, server `mvn -B test` (39 passed, against #525 as 0.0.38; the converter test now also checks the notes and the positions). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Closes #187
What changes
The Verification panel's Expected box and every Found line:
&&at the end of each line except the last. An&&inside parentheses or a string literal stays on its line. Nothing is split when a weaker operator (||,-->,?:) sits outside the parentheses, so a line never looks like a conjunct of the whole expression.? …,: …). This is what makes A1_09 fit.overflow-wrap: anywhere, which gavefailures.siz|e(),compressionN|otExplicitanddisplay(host|) ==. They now usebreak-word, so a line breaks at a space and splits a name only when the name alone is wider than the line.∀host) goes above its predicate, so the predicate gets the whole width. The card and the Found box get less horizontal padding, and the Found lines get line-height 1.4 instead of 1.6.This is display only. The text of every expression is the same as before, only laid out on more lines. The hover and the CLI do not change.
Acceptance check: the six study tasks at the study width
Headless VS Code at 1600×900, Dark Modern, default side bar (300 px; the error card is about 240 px). Verifier 0.0.37, the released one, which is what this branch builds with. Each image shows main on the left and this PR on the right. The webview is 794 px tall. "Found ends at" is the bottom of the Found box, in webview px.
Expected and Found are fully visible in all six, with no name broken.
Verifier version
None needed. This works with 0.0.37 and with main.
Overlap with open PRs
vc-changes.ts(formatImplicationLines, with its ownsplitConjuncts), and this PR changesrenderVCLinein the same file.&&, so a line List Found as facts and show only readable simplification steps #185 has already split is left as it is.splitConjuncts. This PR's copy is inhighlighting.tsand follows the same rules.sections.ts.Not changed
E_OSS_A1_01_SlideRecode|r.java) still break anywhere. They are in a<pre>withbreak-word, and the names are wider than the card. This was outside the issue.||(A1_01's Expected) is not split into lines. The issue asks for conjuncts.Checks
npm run test:unit(127 passed; 3 new tests, each fails on main),npm run test:typecheck,npm run lint, servermvn -B test(39 passed, unchanged).🤖 Generated with Claude Code