From 9a3be5eda85b364d43fb6abda2f9560fcfae59c0 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sat, 20 Jun 2026 00:04:53 -0400 Subject: [PATCH] TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 5: refactor(gui_2): migrate L5920 render_external_editor_panel config to Result[T] (Phase 5) Extract _render_external_editor_panel_config_result helper from the external editor config rendering try/except in render_external_editor_panel. Legacy wrapper drains errors to app._last_request_errors per FR-BC-4 event-handler pattern. [pre-audit] L5920 INTERNAL_BROAD_CATCH [post-audit] V count: 5 -> 4 (L5920 removed) --- src/gui_2.py | 109 +++++++++++++++++++++++-------------- tests/test_gui_2_result.py | 41 ++++++++++++++ 2 files changed, 109 insertions(+), 41 deletions(-) diff --git a/src/gui_2.py b/src/gui_2.py index 43aed784..d9cb4d6a 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -5838,47 +5838,11 @@ def render_external_editor_panel(app: App) -> None: from src.external_editor import get_default_launcher imgui.text("External Editor for Diff Viewing") imgui.separator() - try: - launcher = get_default_launcher(app.config) - editors = launcher.config.editors - default_name = launcher.config.default_editor - if not editors: - imgui.text_colored(C_REQ(), " No editors configured") - imgui.text("") - imgui.text("Add editors in config.toml:") - imgui.text(" [tools.text_editors.vscode]") - imgui.text(' path = "C:\\\\path\\\\to\\\\code.exe"') - imgui.text(' diff_args = ["--diff"]') - imgui.text("") - imgui.text(" [tools.text_editors.notepadpp]") - imgui.text(' path = "C:\\\\path\\\\to\\\\notepad++.exe"') - imgui.text(' diff_args = ["-multiInst", "-nosession"]') - imgui.text("") - imgui.text("Then set default in [tools.default_editor]") - else: - imgui.text("Default Editor:") - editor_names = sorted(list(editors.keys())) - if default_name and default_name in editor_names: current_idx = editor_names.index(default_name) - else: current_idx = 0 - changed, new_idx = imgui.combo("##editor_combo", current_idx, editor_names) - if changed: app._set_external_editor_default(editor_names[new_idx]) - imgui.text("") - imgui.text("Configured Editors:") - imgui.separator() - for name in editor_names: - editor = editors.get(name) - if not editor: continue - is_default = name == default_name - marker = " (default)" if is_default else "" - if is_default: imgui.text_colored(C_IN(), f" {name}{marker}") - else: imgui.text(f" {name}{marker}") - imgui.text(f" {editor.path}") - if editor.diff_args: imgui.textDisabled(f" diff: {editor.diff_args}") - imgui.text("") - imgui.text("Config: config.toml [tools.text_editors]") - imgui.text("Override: manual_slop.toml default_editor") - except Exception as e: - imgui.text_colored(C_TC(), f"Error: {str(e)}") + ext_config_result = _render_external_editor_panel_config_result(app) + if not ext_config_result.ok: + if not hasattr(app, '_last_request_errors'): app._last_request_errors = [] + app._last_request_errors.append(("_render_external_editor_panel_config", ext_config_result.errors[0])) + imgui.text_colored(C_TC(), f"Error: {ext_config_result.errors[0].message}") def render_approve_script_modal(app: App) -> None: """ @@ -8016,6 +7980,69 @@ def _render_text_viewer_window_ced_result(app: "App") -> Result[bool]: original=e, )]) + +def _render_external_editor_panel_config_result(app: "App") -> Result[bool]: + """Drain-aware variant of L5920 render_external_editor_panel config block. + + Extracts the external editor config rendering try/except from + render_external_editor_panel into a Result-returning helper. On success, + returns Result(data=True). On exception, converts the exception to + ErrorInfo and returns Result(data=False, errors=[ErrorInfo]). + + The legacy wrapper drains errors to app._last_request_errors (per FR-BC-4 + event-handler drain pattern; data plane attribute). + + [C: src/gui_2.py:render_external_editor_panel (L5920 legacy wrapper)] + """ + from src.external_editor import get_default_launcher + try: + launcher = get_default_launcher(app.config) + editors = launcher.config.editors + default_name = launcher.config.default_editor + if not editors: + imgui.text_colored(C_REQ(), " No editors configured") + imgui.text("") + imgui.text("Add editors in config.toml:") + imgui.text(" [tools.text_editors.vscode]") + imgui.text(' path = "C:\\\\path\\\\to\\\\code.exe"') + imgui.text(' diff_args = ["--diff"]') + imgui.text("") + imgui.text(" [tools.text_editors.notepadpp]") + imgui.text(' path = "C:\\\\path\\\\to\\\\notepad++.exe"') + imgui.text(' diff_args = ["-multiInst", "-nosession"]') + imgui.text("") + imgui.text("Then set default in [tools.default_editor]") + else: + imgui.text("Default Editor:") + editor_names = sorted(list(editors.keys())) + if default_name and default_name in editor_names: current_idx = editor_names.index(default_name) + else: current_idx = 0 + changed, new_idx = imgui.combo("##editor_combo", current_idx, editor_names) + if changed: app._set_external_editor_default(editor_names[new_idx]) + imgui.text("") + imgui.text("Configured Editors:") + imgui.separator() + for name in editor_names: + editor = editors.get(name) + if not editor: continue + is_default = name == default_name + marker = " (default)" if is_default else "" + if is_default: imgui.text_colored(C_IN(), f" {name}{marker}") + else: imgui.text(f" {name}{marker}") + imgui.text(f" {editor.path}") + if editor.diff_args: imgui.textDisabled(f" diff: {editor.diff_args}") + imgui.text("") + imgui.text("Config: config.toml [tools.text_editors]") + imgui.text("Override: manual_slop.toml default_editor") + return Result(data=True) + except Exception as e: + return Result(data=False, errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=f"Render external editor panel config failed: {e}", + source="gui_2._render_external_editor_panel_config_result", + original=e, + )]) + #endregion: Phase 5 Event Handler Result Helpers #endregion: MMA diff --git a/tests/test_gui_2_result.py b/tests/test_gui_2_result.py index 35555105..8b3fe2a2 100644 --- a/tests/test_gui_2_result.py +++ b/tests/test_gui_2_result.py @@ -1185,3 +1185,44 @@ def test_phase_5_l5786_render_text_viewer_window_ced_result_failure(): err = result.errors[0] assert err.source == "gui_2._render_text_viewer_window_ced_result" assert "ced set_text failed" in err.message + + +def test_phase_5_l5920_render_external_editor_panel_config_result_success(): + """ + L5920 _render_external_editor_panel_config_result returns Result.ok=True on success. + + The helper wraps the external editor config rendering try/except in + render_external_editor_panel. On success, returns Result(data=True). + """ + from src import gui_2 + from unittest.mock import MagicMock, patch + app = MagicMock() + app.config = {"tools": {"text_editors": {}, "default_editor": ""}} + mock_launcher = MagicMock() + mock_launcher.config.editors = {} + mock_launcher.config.default_editor = None + with patch("src.external_editor.get_default_launcher", return_value=mock_launcher), \ + patch.object(gui_2, "imgui", new=MagicMock()): + result = gui_2._render_external_editor_panel_config_result(app) + assert result.ok, f"Expected ok=True on success, got errors: {result.errors}" + assert result.data is True + + +def test_phase_5_l5920_render_external_editor_panel_config_result_failure(): + """ + L5920 _render_external_editor_panel_config_result returns Result.ok=False with ErrorInfo on failure. + + When the external editor config rendering raises, the helper converts the + exception to ErrorInfo and returns Result(data=False, errors=[ErrorInfo]). + """ + from src import gui_2 + from unittest.mock import MagicMock, patch + app = MagicMock() + app.config = {"tools": {"text_editors": {}, "default_editor": ""}} + with patch("src.external_editor.get_default_launcher", side_effect=RuntimeError("ext editor config blew up")): + result = gui_2._render_external_editor_panel_config_result(app) + assert not result.ok, f"Expected ok=False on failure, got data: {result.data}" + assert result.errors, "Expected at least one error on failure" + err = result.errors[0] + assert err.source == "gui_2._render_external_editor_panel_config_result" + assert "ext editor config blew up" in err.message