The property before the text, which is the only order that works - #48
Merged
Merged
Conversation
v0.5.12 did not fix this. It set `use-markup` on the builder, and a builder applies a row's title as the
object is constructed and the property afterwards, whatever order the chain lists them in — so the title
was still parsed as markup, the row still rendered empty, and the property still read `false` at the end.
That last part is why it passed. The check added with it counted declarations, and thirty-two
declarations were present on code that did not work. A test that cannot fail is not a test.
Measured instead, in `examples/markup.rs`:
built with the builder: uses markup: false — and GTK logged "Failed to set text … from markup"
set after construction: uses markup: false — and said nothing at all
So every row is now built by one helper that constructs the row, sets the property, and only then sets the
text. All thirty-two call sites go through it, and the helper lives in a small library beside the binary so
that the example exercises what the window uses rather than a copy that can drift.
Two checks replace the one that was wrong. The structural one refuses any `ActionRow::builder()` in the
window at all, because a builder cannot set the property first. The behavioural one runs the example under
`xvfb`, catches what GTK logs through `g_log_structured` — which is why a log handler saw nothing and the
writer function sees everything — and fails if a row's text could not be set. It was run against the
construction it replaces, and it failed there, before it was trusted here.
`G_DEBUG=fatal-warnings` was the first idea for that and is too blunt: it also aborts on an unrelated
missing gsettings schema, which says nothing about this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`expect_used` is denied outside `#[test]` functions and an example is neither. Without a display there is nothing for this to ask anyway, so it says that and stops rather than panicking with a backtrace in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Shipping as v0.5.13. v0.5.12 did not fix the bug it claimed to, and the check that shipped with it was satisfied by the broken code.
What was wrong with the fix
use_markup(false)on the builder does not work. A builder applies a row's title as the object is constructed and the property afterwards, whatever order the chain lists them in. So the title was still parsed as markup, the row still rendered empty — and the property still readfalseat the end, which is exactly why a check that asked the property passed.Measured rather than reasoned about, in
examples/markup.rs:A test that cannot fail is not a test. The v0.5.12 check counted declarations: thirty-two rows, thirty-two
use_markup(false), all present on code that did not work.The fix
One helper that constructs the row, sets the property, and only then sets the text. All thirty-two call sites go through it. It lives in a small library beside the binary so the example exercises what the window uses rather than a copy of it that can drift.
Two checks, replacing the one that was wrong
adw::ActionRow::builder()may appear in the window at all, because a builder cannot set the property first. That is an invariant about construction rather than a count of words.xvfb, installs a writer function — GTK 4 logs throughg_log_structured, which is why a log handler saw nothing and the writer sees everything — and fails if any row's text could not be set.The behavioural one was run against the construction it replaces, and it failed there, before it was trusted here:
G_DEBUG=fatal-warningswas the first idea and is too blunt — it also aborts on an unrelated missing gsettings schema, which says nothing about this.🤖 Generated with Claude Code