Skip to content

fix: Add sort to prevent scope deleted by former iterations in for loop#1440

Open
liegle wants to merge 1 commit into
elkowar:masterfrom
liegle:loopfix
Open

fix: Add sort to prevent scope deleted by former iterations in for loop#1440
liegle wants to merge 1 commit into
elkowar:masterfrom
liegle:loopfix

Conversation

@liegle

@liegle liegle commented May 27, 2026

Copy link
Copy Markdown
Contributor

Description

This pr aims to fix unexpected "Scope not in graph" error logs like this issue

Cause of the error log

In notify_value_changed, when iterating affected_subsopes, listeners of those scopes processed first will remove scopes to be processed later, if that "later scope" is subscope of both current scope_index and one affected_subscope iterated first, and this affected_subscope is a for-loop scope.
An example:

(defvar TEST '{"value":1,"array":[1,2,3]}')
(defwidget test-inner [v]
    (label
        :text { TEST.value }))
(defwindow test
    ...
    (box
        (for v in { TEST.array }
            (test-inner
                :v v))))

Scope relationship:

0(root) -> 1(box)
1 -> 2(for element 1)
0 -> 3(label 1)
1 -> 4(for element 2)
0 -> 5(label 2)
1 -> 6(for element 3)
0 -> 7(label 3)

When calling tree.notify_value_changed(0, "TEST"), 1, 3, 5, 7 will be in affected_subscopes because they all reference TEST. When 1 is iterated, 3, 5, 7 will be removed, if any of them is iterated after this, the error log will be printed and the iteration will terminate. This will cause latter unremoved things impossible to be processed.

Solution

Sort affected_subscopes before iterating it, as scopes created latter will have larger indices.

Additional

Maybe a better way to fix it is not to set superscopes of widget definitions to root scope in build_basic_gtk_widget. But I don't known why it was written like this and wonder if is it good to change it.

Checklist

  • All widgets I've added are correctly documented.
    no widgets added
  • I added my changes to CHANGELOG.md, if appropriate.
    no need
  • The documentation in the docs/content/main directory has been adjusted to reflect my changes.
    no need
  • I used cargo fmt to automatically format all code before committing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant