From 365ac0846e8f167d57091c1332349d54fb2ca1be Mon Sep 17 00:00:00 2001 From: Dusten Hubbard Date: Tue, 21 Jul 2026 18:39:29 -0500 Subject: [PATCH] feat(gui): report actual sections in copy-to feedback The "Copy to sections" result previously reported only a count ("copied to N sections"). Report the actual section numbers that received the trace(s) instead, so the message reflects what was truly done rather than what was requested -- a self-check that surfaces any silently-skipped targets. Long contiguous runs collapse to ranges (e.g. "2-5, 10") and the message uses singular/plural grammar ("section 5" vs "sections 2, 3"). Existing non-invertible "skipped" reporting is preserved. Message building moves into a pure format_copy_result helper so it can be unit-tested. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LiM1igjbwUJfVBgR7Y31Kp --- PyReconstruct/modules/gui/dialog/__init__.py | 2 +- .../modules/gui/dialog/copy_to_sections.py | 67 ++++++++++++++++++ .../modules/gui/main/field_widget_2_trace.py | 21 +++--- tests/test_copy_traces_to_sections.py | 68 +++++++++++++++++++ 4 files changed, 144 insertions(+), 14 deletions(-) diff --git a/PyReconstruct/modules/gui/dialog/__init__.py b/PyReconstruct/modules/gui/dialog/__init__.py index 84bda0c4..06f3f6d5 100644 --- a/PyReconstruct/modules/gui/dialog/__init__.py +++ b/PyReconstruct/modules/gui/dialog/__init__.py @@ -10,7 +10,7 @@ from .shapes import ShapesDialog from .trace_palette import TracePaletteDialog from .quick_dialog import QuickDialog, QuickTabDialog -from .copy_to_sections import CopyToSectionsDialog +from .copy_to_sections import CopyToSectionsDialog, format_copy_result from .flag import FlagDialog from .file_dialog import FileDialog from .all_options import AllOptionsDialog diff --git a/PyReconstruct/modules/gui/dialog/copy_to_sections.py b/PyReconstruct/modules/gui/dialog/copy_to_sections.py index 200b9081..2ba91985 100644 --- a/PyReconstruct/modules/gui/dialog/copy_to_sections.py +++ b/PyReconstruct/modules/gui/dialog/copy_to_sections.py @@ -83,6 +83,73 @@ def parse_section_spec(text : str, valid_sections) -> tuple: return chosen, bad, missing +def format_section_run(numbers) -> str: + """Format section numbers as a compact, readable list. + + A run of three or more consecutive numbers is collapsed to "lo-hi" + (e.g. [2, 3, 4, 5, 10] -> "2-5, 10"); singletons and pairs stay listed + plainly so a short run like "2, 3" is not obscured by a range. + + Params: + numbers (iterable): the section numbers to render + Returns: + (str): the formatted list, "" for no numbers + """ + nums = sorted(set(numbers)) + if not nums: + return "" + + runs = [] + start = prev = nums[0] + for n in nums[1:]: + if n == prev + 1: + prev = n + else: + runs.append((start, prev)) + start = prev = n + runs.append((start, prev)) + + parts = [] + for lo, hi in runs: + if hi - lo >= 2: # 3+ consecutive: collapse to a range + parts.append(f"{lo}-{hi}") + else: # single number or a pair: list plainly + parts.extend(str(n) for n in range(lo, hi + 1)) + return ", ".join(parts) + + +def format_copy_result(copied_to, skipped, excluded_current=None) -> str: + """Build the user-facing summary for a "Copy to sections" run. + + Reports the sections that ACTUALLY received the trace(s) rather than a bare + count, so the message reflects what was done (a self-check), not what was + typed. Existing non-invertible "skipped" reporting is preserved, and the + excluded current section is noted when applicable. + + Params: + copied_to (iterable): section numbers that received the trace(s) + skipped (iterable): section numbers skipped (non-invertible tform) + excluded_current (int): the current section left unchanged, or None + Returns: + (str): the message to show, "" if there is nothing to report + """ + msgs = [] + if copied_to: + unique = sorted(set(copied_to)) + noun = "section" if len(unique) == 1 else "sections" + msgs.append(f"Copied trace(s) to {noun} {format_section_run(unique)}.") + if skipped: + msgs.append( + "Skipped section(s) with a non-invertible transform: " + + ", ".join(str(n) for n in sorted(skipped)) + ) + if excluded_current is not None: + msgs.append( + f"The current section ({excluded_current}) was left unchanged." + ) + return "\n".join(msgs) + + class CopyToSectionsDialog(QDialog): """Pick the target sections to copy the selected trace(s) onto.""" diff --git a/PyReconstruct/modules/gui/main/field_widget_2_trace.py b/PyReconstruct/modules/gui/main/field_widget_2_trace.py index 38664644..837bca73 100644 --- a/PyReconstruct/modules/gui/main/field_widget_2_trace.py +++ b/PyReconstruct/modules/gui/main/field_widget_2_trace.py @@ -13,6 +13,7 @@ ShapesDialog, ObjectGroupDialog, CopyToSectionsDialog, + format_copy_result, ) from PyReconstruct.modules.gui.utils import notify from PyReconstruct.modules.calc import ( @@ -930,19 +931,13 @@ def copyTracesToSections(self, traces : list): self.table_manager.updateObjects(names) self.reload() - # report the outcome to the user - msgs = [] - if copied_to: - msgs.append(f"Copied trace(s) to {len(copied_to)} section(s).") - if skipped: - msgs.append( - "Skipped section(s) with a non-invertible transform: " - + ", ".join(str(n) for n in sorted(skipped)) - ) - if excluded_current: - msgs.append(f"The current section ({current}) was left unchanged.") - if msgs: - notify("\n".join(msgs)) + # report the outcome to the user, listing the sections that ACTUALLY + # received the trace(s) so the message reflects what was done + message = format_copy_result( + copied_to, skipped, current if excluded_current else None + ) + if message: + notify(message) return bool(copied_to) diff --git a/tests/test_copy_traces_to_sections.py b/tests/test_copy_traces_to_sections.py index 68abe805..fabf4e3b 100644 --- a/tests/test_copy_traces_to_sections.py +++ b/tests/test_copy_traces_to_sections.py @@ -164,6 +164,74 @@ def test_parse_section_spec_exotic_unicode_digits_do_not_crash(): assert chosen == {2} and bad == ["5²"] +# --------------------------------------------------------------------------- +# result message formatting +# --------------------------------------------------------------------------- + +def test_format_section_run_collapses_only_long_runs(): + from PyReconstruct.modules.gui.dialog.copy_to_sections import format_section_run + + assert format_section_run([]) == "" + assert format_section_run([5]) == "5" + # a pair stays a plain list (not obscured as a range) + assert format_section_run([2, 3]) == "2, 3" + # 3+ consecutive collapse to lo-hi + assert format_section_run([2, 3, 4, 5, 10, 17]) == "2-5, 10, 17" + # unsorted / duplicate input is normalised + assert format_section_run([17, 2, 10, 3, 3, 5, 4]) == "2-5, 10, 17" + # multiple long runs + assert format_section_run([1, 2, 3, 7, 8, 9]) == "1-3, 7-9" + + +def test_message_lists_actual_sections_not_a_count(): + """The success line must name the sections that were written, not just how + many (the whole point of the feedback change).""" + from PyReconstruct.modules.gui.dialog.copy_to_sections import format_copy_result + + msg = format_copy_result([2, 3, 10, 17], []) + assert "Copied trace(s) to sections 2, 3, 10, 17." in msg + # no bare count phrasing survives + assert "4 section" not in msg + + +def test_message_singular_vs_plural_grammar(): + from PyReconstruct.modules.gui.dialog.copy_to_sections import format_copy_result + + single = format_copy_result([5], []) + assert single == "Copied trace(s) to section 5." + plural = format_copy_result([5, 6], []) + assert plural == "Copied trace(s) to sections 5, 6." + + +def test_message_reports_only_sections_actually_written(): + """When some targets are skipped (non-invertible), the success line lists + only the sections that ACTUALLY received the trace, and the skipped ones are + reported separately.""" + from PyReconstruct.modules.gui.dialog.copy_to_sections import format_copy_result + + # requested {2, 3, 4}; section 3 was skipped -> only 2 and 4 written + msg = format_copy_result([2, 4], [3]) + assert "Copied trace(s) to sections 2, 4." in msg + assert "3" not in msg.split("\n")[0] # 3 is NOT in the success line + assert "Skipped section(s) with a non-invertible transform: 3" in msg + + +def test_message_empty_result_is_blank(): + from PyReconstruct.modules.gui.dialog.copy_to_sections import format_copy_result + + assert format_copy_result([], []) == "" + + +def test_message_includes_excluded_current_and_collapses_large_lists(): + from PyReconstruct.modules.gui.dialog.copy_to_sections import format_copy_result + + written = list(range(10, 26)) # 10..25 contiguous + msg = format_copy_result(written, [], excluded_current=7) + lines = msg.split("\n") + assert lines[0] == "Copied trace(s) to sections 10-25." + assert lines[-1] == "The current section (7) was left unchanged." + + # --------------------------------------------------------------------------- # data-model copy # ---------------------------------------------------------------------------