Private
Public Access
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)
This commit is contained in:
+68
-41
@@ -5838,47 +5838,11 @@ def render_external_editor_panel(app: App) -> None:
|
|||||||
from src.external_editor import get_default_launcher
|
from src.external_editor import get_default_launcher
|
||||||
imgui.text("External Editor for Diff Viewing")
|
imgui.text("External Editor for Diff Viewing")
|
||||||
imgui.separator()
|
imgui.separator()
|
||||||
try:
|
ext_config_result = _render_external_editor_panel_config_result(app)
|
||||||
launcher = get_default_launcher(app.config)
|
if not ext_config_result.ok:
|
||||||
editors = launcher.config.editors
|
if not hasattr(app, '_last_request_errors'): app._last_request_errors = []
|
||||||
default_name = launcher.config.default_editor
|
app._last_request_errors.append(("_render_external_editor_panel_config", ext_config_result.errors[0]))
|
||||||
if not editors:
|
imgui.text_colored(C_TC(), f"Error: {ext_config_result.errors[0].message}")
|
||||||
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)}")
|
|
||||||
|
|
||||||
def render_approve_script_modal(app: App) -> None:
|
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,
|
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: Phase 5 Event Handler Result Helpers
|
||||||
|
|
||||||
#endregion: MMA
|
#endregion: MMA
|
||||||
|
|||||||
@@ -1185,3 +1185,44 @@ def test_phase_5_l5786_render_text_viewer_window_ced_result_failure():
|
|||||||
err = result.errors[0]
|
err = result.errors[0]
|
||||||
assert err.source == "gui_2._render_text_viewer_window_ced_result"
|
assert err.source == "gui_2._render_text_viewer_window_ced_result"
|
||||||
assert "ced set_text failed" in err.message
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user