From 589afa9e6655b3048f32cd9cd28c72b02d737b1a Mon Sep 17 00:00:00 2001 From: timrid <6593626+timrid@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:43:08 +0200 Subject: [PATCH] Fix collapsed entries reopening after edits --- CHANGELOG.md | 4 ++ construct_editor/core/construct_editor.py | 38 ++++------- .../gallery/test_array_with_bit_struct.py | 39 +++++++++++ construct_editor/main.py | 30 +++------ .../test_wx_construct_hex_editor.py | 67 ++++++++++++++++--- 5 files changed, 120 insertions(+), 58 deletions(-) create mode 100644 construct_editor/gallery/test_array_with_bit_struct.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 341e328..af69f04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## UNRELEASED +**Bugfixes:** +- Fixed collapsed entries unexpectedly expanding after editing another entry when their hidden children were still expanded. + ## [0.2.0] - 2026-07-30 **Breaking changes:** - Updated construct-typing dependency to v0.8.1+ and updated the DataClass definitions to reflect the breaking changes. ([#43](https://github.com/timrid/construct-editor/pull/43), [#42](https://github.com/timrid/construct-editor/pull/42)) diff --git a/construct_editor/core/construct_editor.py b/construct_editor/core/construct_editor.py index d87360a..f48fca6 100644 --- a/construct_editor/core/construct_editor.py +++ b/construct_editor/core/construct_editor.py @@ -17,9 +17,7 @@ def __init__(self, construct: cs.Construct[t.Any, t.Any], model: ConstructEditor self.change_construct(construct) - self.on_entry_selected: CallbackList[ - [entries.EntryConstruct] - ] = CallbackList() + self.on_entry_selected: CallbackList[[entries.EntryConstruct]] = CallbackList() self.on_root_obj_changed: CallbackList[[t.Any]] = CallbackList() @abc.abstractmethod @@ -130,9 +128,7 @@ def change_construct(self, constr: cs.Construct[t.Any, t.Any]) -> None: self._construct = include_metadata(constr) # create entry from the construct - self._model.root_entry = entries.create_entry_from_construct( - self._model, None, self._construct, None, "" - ) + self._model.root_entry = entries.create_entry_from_construct(self._model, None, self._construct, None, "") self._model.list_viewed_entries.clear() @@ -152,9 +148,7 @@ def parse(self, binary: bytes, **contextkw: t.Any): self._model.root_obj = self._construct.parse(binary, **contextkw) self.show_parse_error_message(None, None) except Exception as e: - self.show_parse_error_message( - f"Error while parsing binary data: {type(e).__name__}\n{str(e)}", e - ) + self.show_parse_error_message(f"Error while parsing binary data: {type(e).__name__}\n{str(e)}", e) self._model.root_obj = None # clear all commands, when new data is set from external @@ -169,9 +163,7 @@ def build(self, **contextkw: t.Any) -> bytes: binary = self._construct.build(self._model.root_obj, **contextkw) self.show_build_error_message(None, None) except Exception as e: - self.show_build_error_message( - f"Error while building binary data: {type(e).__name__}\n{str(e)}", e - ) + self.show_build_error_message(f"Error while building binary data: {type(e).__name__}\n{str(e)}", e) raise e # Parse the build binary, so that constructs that parses from nothing @@ -264,18 +256,16 @@ def restore_expansion_from_model(self, entry: entries.EntryConstruct): if visible_entry is None: return - if visible_entry.row_expanded is True: - self.expand_entry(visible_entry) - else: - self.collapse_entry(visible_entry) - + row_expanded = visible_entry.row_expanded subentries = self._model.get_children(entry) - if len(subentries) == 0: - return - for subentry in subentries: self.restore_expansion_from_model(subentry) + if row_expanded is True: + self.expand_entry(visible_entry) + else: + self.collapse_entry(visible_entry) + def enable_list_view(self, entry: entries.EntryConstruct): """ Enable the list view for an entry. @@ -360,9 +350,7 @@ def _get_list_viewed_column_count(self): column_count = max(column_count, len(flat_list)) return column_count - def _get_list_viewed_column_names( - self, selected_entry: entries.EntryConstruct - ) -> t.List[str]: + def _get_list_viewed_column_names(self, selected_entry: entries.EntryConstruct) -> t.List[str]: """ Get the names of all list viewed columns. @@ -374,9 +362,7 @@ def _get_list_viewed_column_names( flat_list = self._model.create_flat_subentry_list(selected_entry) for entry in flat_list: column_path = entry.path - column_path = column_path[ - len(selected_entry.path) : - ] # remove the path from the selected_entry + column_path = column_path[len(selected_entry.path) :] # remove the path from the selected_entry column_names.append(entries.create_path_str(column_path)) return column_names diff --git a/construct_editor/gallery/test_array_with_bit_struct.py b/construct_editor/gallery/test_array_with_bit_struct.py new file mode 100644 index 0000000..17c2dce --- /dev/null +++ b/construct_editor/gallery/test_array_with_bit_struct.py @@ -0,0 +1,39 @@ +import dataclasses + +import construct as cs +import construct_typed as cst + +from construct_editor.gallery import GalleryItem + + +# Standard example structure ############################################################################################### +@dataclasses.dataclass +class Record(cst.DataclassMixin): + @dataclasses.dataclass + class Flags(cst.DataclassMixin): + flag_1: bool = cst.csfield(cs.Flag) + flag_2: bool = cst.csfield(cs.Flag) + flag_3: bool = cst.csfield(cs.Flag) + flag_4: bool = cst.csfield(cs.Flag) + flag_5: bool = cst.csfield(cs.Flag) + flag_6: bool = cst.csfield(cs.Flag) + flag_7: bool = cst.csfield(cs.Flag) + flag_8: bool = cst.csfield(cs.Flag) + + flags: Flags = cst.csfield( + cs.ByteSwapped(cst.DataclassBitStruct(Flags, reverse=True)), + ) + + +constr = cs.GreedyRange(cst.DataclassStruct(Record)) + + +_sample_record = constr.parse(bytes(constr.subcon.sizeof()))[0] +sample_records = [dataclasses.replace(_sample_record) for _ in range(2)] + +gallery_item = GalleryItem( + construct=constr, + example_binarys={ + "Sample": constr.build(sample_records), + }, +) diff --git a/construct_editor/main.py b/construct_editor/main.py index 50dbcc1..5e6c5e8 100644 --- a/construct_editor/main.py +++ b/construct_editor/main.py @@ -13,6 +13,7 @@ import construct_editor.gallery.example_pe32coff import construct_editor.gallery.test_aligned import construct_editor.gallery.test_array +import construct_editor.gallery.test_array_with_bit_struct import construct_editor.gallery.test_bits_swapped_bitwise import construct_editor.gallery.test_bitwise import construct_editor.gallery.test_bytes_greedybytes @@ -119,6 +120,7 @@ def __init__(self, parent: ConstructGalleryFrame): "## arrays ranges and repeaters ######": None, "Test: Array": construct_editor.gallery.test_array.gallery_item, "Test: GreedyRange": construct_editor.gallery.test_greedyrange.gallery_item, + "Test: Array with BitStruct": construct_editor.gallery.test_array_with_bit_struct.gallery_item, # "Test: RepeatUntil (TODO)": None, "## specials ##########################": None, "Test: Renamed": construct_editor.gallery.test_renamed.gallery_item, @@ -203,18 +205,14 @@ def __init__(self, parent: ConstructGalleryFrame): vsizer.Add(self.gallery_selector_lbx, 1, wx.ALL, 1) # reload btn - self.reload_btn = wx.Button( - self, wx.ID_ANY, "Reload", wx.DefaultPosition, wx.DefaultSize, 0 - ) + self.reload_btn = wx.Button(self, wx.ID_ANY, "Reload", wx.DefaultPosition, wx.DefaultSize, 0) vsizer.Add(self.reload_btn, 0, wx.ALL | wx.EXPAND, 1) # line vsizer.Add(wx.StaticLine(self), 0, wx.TOP | wx.BOTTOM | wx.EXPAND, 5) # clear binary - self.clear_binary_btn = wx.Button( - self, wx.ID_ANY, "Clear Binary", wx.DefaultPosition, wx.DefaultSize, 0 - ) + self.clear_binary_btn = wx.Button(self, wx.ID_ANY, "Clear Binary", wx.DefaultPosition, wx.DefaultSize, 0) vsizer.Add(self.clear_binary_btn, 0, wx.ALL | wx.EXPAND, 1) # example selctor @@ -228,9 +226,7 @@ def __init__(self, parent: ConstructGalleryFrame): name="gallery_selector", ) if len(default_gallery_item.example_binarys) > 0: - self.example_selector_lbx.SetStringSelection( - list(default_gallery_item.example_binarys.keys())[0] - ) + self.example_selector_lbx.SetStringSelection(list(default_gallery_item.example_binarys.keys())[0]) vsizer.Add(self.example_selector_lbx, 0, wx.ALL | wx.EXPAND, 1) # load binary from file @@ -265,14 +261,10 @@ def __init__(self, parent: ConstructGalleryFrame): self.SetSizer(self.sizer) # Connect Events ################################################## - self.gallery_selector_lbx.Bind( - wx.EVT_LISTBOX, self.on_gallery_selection_changed - ) + self.gallery_selector_lbx.Bind(wx.EVT_LISTBOX, self.on_gallery_selection_changed) self.reload_btn.Bind(wx.EVT_BUTTON, self.on_gallery_selection_changed) self.clear_binary_btn.Bind(wx.EVT_BUTTON, self.on_clear_binary_clicked) - self.example_selector_lbx.Bind( - wx.EVT_LISTBOX, self.on_example_selection_changed - ) + self.example_selector_lbx.Bind(wx.EVT_LISTBOX, self.on_example_selection_changed) self.load_binary_file_btn.Bind(wx.EVT_BUTTON, self.on_load_binary_file_clicked) @@ -291,12 +283,8 @@ def on_gallery_selection_changed(self, event): self.example_selector_lbx.Clear() if len(gallery_item.example_binarys) > 0: - self.example_selector_lbx.InsertItems( - list(gallery_item.example_binarys.keys()), 0 - ) - self.example_selector_lbx.SetStringSelection( - list(gallery_item.example_binarys.keys())[0] - ) + self.example_selector_lbx.InsertItems(list(gallery_item.example_binarys.keys()), 0) + self.example_selector_lbx.SetStringSelection(list(gallery_item.example_binarys.keys())[0]) example = self.example_selector_lbx.GetStringSelection() example_binary = gallery_item.example_binarys[example] diff --git a/tests/wx_integration/test_wx_construct_hex_editor.py b/tests/wx_integration/test_wx_construct_hex_editor.py index 8aa865a..8eb1bbb 100644 --- a/tests/wx_integration/test_wx_construct_hex_editor.py +++ b/tests/wx_integration/test_wx_construct_hex_editor.py @@ -1,18 +1,63 @@ -"""Tests for construct_editor.wx_widgets.wx_construct_hex_editor.WxConstructHexEditor. +"""Integration tests for the composite Construct and hex editor.""" -Covers the composite WxConstructHexEditor widget. -""" +# pyright: reportPrivateUsage=false +import pytest +import wx +from construct_editor.core.model import ConstructEditorColumn +from construct_editor.gallery.test_array_with_bit_struct import gallery_item +from construct_editor.wx_widgets.wx_construct_hex_editor import WxConstructHexEditor +from tests.wx_integration.wx_test_helpers import ( + WxTestHarness, + construct_editor_cell_screen_rect, +) -def test_placeholder(wx_app_and_ui_sim) -> None: - pass +@pytest.mark.parametrize("collapse_flags_first", [False, True], ids=["expanded-flags", "collapsed-flags"]) +def test_editing_flag_preserves_collapsed_sibling(wx_harness: WxTestHarness, collapse_flags_first: bool) -> None: + panel = WxConstructHexEditor(wx_harness.frame, gallery_item.construct, binary=gallery_item.example_binarys["Sample"]) + editor = panel.construct_editor + sizer = wx.BoxSizer(wx.VERTICAL) + sizer.Add(panel, 1, wx.EXPAND) + wx_harness.frame.SetSizer(sizer) + wx_harness.frame.SetSize(wx.Size(1200, 800)) + wx_harness.frame.Layout() + editor.expand_all() + wx_harness.wait_ms(100) -# TODO: Tests to add: -# - Widget can be constructed and shown without errors -# - Setting binary data triggers a parse pass -# - Editing a struct field via the construct editor updates the hex view -# - toggle_hex_visibility() hides/shows the hex editor pane -# - round-trip: parse bytes -> build -> compare to original bytes + model = editor._model + root = model.root_entry + assert root is not None + first, second = model.get_children(root) + first_flags = model.get_children(first)[0] + second_flags = model.get_children(second)[0] + flag = model.get_children(second_flags)[0] + first_item = model.entry_to_dvc_item(first) + assert editor._dvc.IsExpanded(first_item) + assert editor._dvc.IsExpanded(model.entry_to_dvc_item(first_flags)) + assert editor._dvc.IsExpanded(model.entry_to_dvc_item(second_flags)) + if collapse_flags_first: + editor.collapse_entry(first_flags) + editor.collapse_entry(first) + wx_harness.wait_ms(100) + assert not editor._dvc.IsExpanded(first_item) + assert first_flags.row_expanded is not collapse_flags_first + + rect = construct_editor_cell_screen_rect(editor, flag, ConstructEditorColumn.Value) + checkbox_size = wx.RendererNative.Get().GetCheckBoxSize(editor._dvc) + wx_harness.move_mouse_to(wx.Point(rect.x + checkbox_size.width // 2, rect.y + rect.height // 2)) + wx_harness.click() + + assert editor.get_root_obj()[1].flags.flag_1 == 1 + assert editor.get_root_obj()[0].flags.flag_1 == 0 + assert panel.hex_panel.hex_editor.binary == b"\x00\x01" + assert not editor._dvc.IsExpanded(first_item) + assert editor._dvc.IsExpanded(model.entry_to_dvc_item(second)) + assert editor._dvc.IsExpanded(model.entry_to_dvc_item(second_flags)) + + assert first.row_expanded is False + assert first_flags.row_expanded is not collapse_flags_first + editor.expand_entry(first) + assert editor._dvc.IsExpanded(model.entry_to_dvc_item(first_flags)) is not collapse_flags_first