Make the layout spy report readable and its warnings trustworthy - #2423
Open
vogella wants to merge 2 commits into
Open
Make the layout spy report readable and its warnings trustworthy#2423vogella wants to merge 2 commits into
vogella wants to merge 2 commits into
Conversation
The layout section of the widget details and of the copied widget tree stayed empty for composites without a layout, which reads like a defect in the spy rather than like information about the widget. Print getLayout() == null instead, mirroring the getLayoutData() == null line right above it, and leave the section out entirely for leaf controls, where it only repeated that the control is not a composite. Also fix the truncation check for grid layouts, which intersected the parent bounds with themselves instead of with the child bounds. The intersection was therefore always equal to the child rectangle and the warnings about missing grab flags could never appear.
Test Results 125 files ±0 125 suites ±0 35m 47s ⏱️ + 6m 39s For more details on these failures, see this check. Results for commit 48492f1. ± Comparison against base commit ae96885. ♻️ This comment has been updated with latest results. |
The copied widget report was dominated by lines that repeat on every node, and the warnings next to them fired often enough on intended states that they could not be followed. Both together made the report hard to use for the bug reports it exists for. Warnings now only fire where something is actually wrong. isVisible() is transitive, so a single hidden ancestor used to stamp its warning onto every descendant; it is now reported once at the node that is really hidden. A control excluded from its layout or sitting behind the topControl of a StackLayout is reported as a note instead of as three warnings about being invisible, having no size and being smaller than its preferred size. A vertically scrollable control is no longer warned about being smaller than its content, which is what scrolling means. All applicable reasons are collected instead of only the first, and the geometric warnings name the sibling they overlap and the bounds they fall outside of. Every composite now reports what its children ask for against the room they are given. A layout error shows up there before it shows up on any single control, and unlike the per-control checks it names the composite to fix rather than the widget that came out wrong. It is also the only check that works for a manually positioned composite, whose computeSize says nothing about its content. The repeated lines are gone: the font only when it differs from the parent, the hint adjustments only when non-zero, the three computeSize lines only when they disagree with the assigned bounds, and the layout data only when the layout of the parent actually reads it. The report starts with the environment it was taken in, because pixel values cannot be interpreted without the scaling factor and the windowing backend, followed by the control count and the warnings grouped by kind. Every node carries an address such as @.3.1 that the summary points at. Widgets are identifiable now: the node line carries the widget text, the owning e4 model element and the CSS class and id, tab folders and tool bars list their items, and style bits are rendered as SWT constant names. Names are looked up per widget class because the bits are not globally unique, for example SWT.H_SCROLL and SWT.HORIZONTAL share one bit. Layout references such as StackLayout.topControl resolve to a child index, orientation constants are named, and identity hashes are stripped so two reports can be compared with a diff.
vogella
force-pushed
the
layout-spy-report
branch
from
August 16, 2026 08:24
4043ae7 to
48492f1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The widget report copied out of the layout spy was mostly boilerplate, and the warnings next to it fired on intended states often enough that they could not be followed. This reworks both sides.
Warnings now only appear where something is actually wrong:
isVisible()is transitive, so one hidden ancestor used to stamp its warning onto every descendant and is now reported once at the node that is really hidden; a control excluded from its layout or sitting behind thetopControlof aStackLayoutbecomes a note rather than three warnings; and a vertically scrollable control is no longer warned about being smaller than its content. All applicable reasons are collected instead of only the first, and the geometric warnings name the sibling they overlap and the bounds they fall outside of. The lines that repeated on every node (font, hint adjustments, the threecomputeSizelines, null layout data under aFillLayout) only appear when they carry information.Every composite additionally reports what its children ask for against the room they get. That is where a layout error becomes visible first, it names the composite to fix instead of the widget that came out wrong, and it is the only check that works for a manually positioned composite, whose own
computeSizesays nothing about its content.The report also gained the things a reader needs to act on it: an environment header, because pixel values mean nothing without the scaling factor and the windowing backend, a summary with the control count and the warnings grouped by kind, and an address per node such as
@.3.1that the summary points at. Widgets are identifiable through their text, the owning e4 model element and their CSS class and id, tab folders and tool bars list their items, and style bits are rendered as SWT constant names, looked up per widget class because the bits are not globally unique.Builds on #2422 and should be merged after it.