Private
Public Access
refactor(external_editor): merge TextEditorConfig + ExternalEditorConfig from models.py
Per the 4-criteria decision rule: editor configs fail C1 (only used by
the editor subsystem), fail C2 (no state machine), fail C3 (no
dedicated test file), borderline C4. MERGE into the existing
src/external_editor.py which already has ExternalEditorLauncher +
the helper functions.
This commit:
1. Adds TextEditorConfig + ExternalEditorConfig + EMPTY_TEXT_EDITOR_CONFIG
class definitions to src/external_editor.py at the top.
2. Removes the same class defs from src/models.py.
3. Adds lazy re-export via the existing __getattr__ in src/models.py
(EAGER would cycle: external_editor was previously importing from
models; if models re-exports, the cycle would deadlock on initial
load).
4. Updates external_editor.py imports to no longer import from models
(the class defs are now local).
Verification: VC8 (TextEditorConfig + ExternalEditorConfig)
from src.external_editor import TextEditorConfig, ExternalEditorConfig,
EMPTY_TEXT_EDITOR_CONFIG # OK
from src.models import TextEditorConfig, ExternalEditorConfig,
EMPTY_TEXT_EDITOR_CONFIG # OK (lazy)
identity check: True for all 3
Tests verified (22/22 PASS):
tests/test_external_editor.py (17 tests)
tests/test_external_editor_gui.py (5 tests)
This commit is contained in:
+57
-4
@@ -6,11 +6,64 @@ import subprocess
|
|||||||
import tempfile
|
import tempfile
|
||||||
|
|
||||||
# TODO(Ed): Eliminate these?
|
# TODO(Ed): Eliminate these?
|
||||||
from pathlib import Path
|
from dataclasses import dataclass, field
|
||||||
from typing import Optional, List, Dict, Any
|
from pathlib import Path
|
||||||
|
from typing import Optional, List, Dict, Any
|
||||||
|
|
||||||
from src.models import ExternalEditorConfig, TextEditorConfig
|
from src.result_types import ErrorInfo, ErrorKind, Result
|
||||||
from src.result_types import ErrorInfo, ErrorKind, Result
|
from src.type_aliases import Metadata
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class TextEditorConfig:
|
||||||
|
name: str = ""
|
||||||
|
path: str = ""
|
||||||
|
diff_args: List[str] = field(default_factory=list)
|
||||||
|
|
||||||
|
def to_dict(self) -> Metadata:
|
||||||
|
return {
|
||||||
|
"name": self.name,
|
||||||
|
"path": self.path,
|
||||||
|
"diff_args": self.diff_args,
|
||||||
|
}
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def from_dict(cls, data: Metadata) -> "TextEditorConfig":
|
||||||
|
return cls(
|
||||||
|
name = data["name"],
|
||||||
|
path = data["path"],
|
||||||
|
diff_args = data.get("diff_args", []),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class ExternalEditorConfig:
|
||||||
|
editors: Dict[str, TextEditorConfig] = field(default_factory=dict)
|
||||||
|
default_editor: Optional[str] = None
|
||||||
|
|
||||||
|
def get_default(self) -> TextEditorConfig:
|
||||||
|
if self.default_editor and self.default_editor in self.editors:
|
||||||
|
return self.editors[self.default_editor]
|
||||||
|
if self.editors:
|
||||||
|
return next(iter(self.editors.values()))
|
||||||
|
return EMPTY_TEXT_EDITOR_CONFIG
|
||||||
|
|
||||||
|
def to_dict(self) -> Metadata:
|
||||||
|
return {
|
||||||
|
"editors": {k: v.to_dict() for k, v in self.editors.items()},
|
||||||
|
"default_editor": self.default_editor,
|
||||||
|
}
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def from_dict(cls, data: Metadata) -> "ExternalEditorConfig":
|
||||||
|
editors = {}
|
||||||
|
for name, ed_data in data.get("editors", {}).items():
|
||||||
|
if isinstance(ed_data, dict): editors[name] = TextEditorConfig.from_dict(ed_data)
|
||||||
|
elif isinstance(ed_data, str): editors[name] = TextEditorConfig(name=name, path=ed_data)
|
||||||
|
return cls(editors=editors, default_editor=data.get("default_editor"))
|
||||||
|
|
||||||
|
|
||||||
|
EMPTY_TEXT_EDITOR_CONFIG: TextEditorConfig = TextEditorConfig()
|
||||||
|
|
||||||
|
|
||||||
class ExternalEditorLauncher:
|
class ExternalEditorLauncher:
|
||||||
|
|||||||
+13
-65
@@ -280,6 +280,11 @@ def __getattr__(name: str) -> Any:
|
|||||||
from src.tool_bias import BiasProfile as _BiasProfile
|
from src.tool_bias import BiasProfile as _BiasProfile
|
||||||
globals()[name] = _BiasProfile
|
globals()[name] = _BiasProfile
|
||||||
return _BiasProfile
|
return _BiasProfile
|
||||||
|
if name in ("TextEditorConfig", "ExternalEditorConfig", "EMPTY_TEXT_EDITOR_CONFIG"):
|
||||||
|
from src.external_editor import TextEditorConfig as _TEC, ExternalEditorConfig as _EEC, EMPTY_TEXT_EDITOR_CONFIG as _ETEC
|
||||||
|
val = {"TextEditorConfig": _TEC, "ExternalEditorConfig": _EEC, "EMPTY_TEXT_EDITOR_CONFIG": _ETEC}[name]
|
||||||
|
globals()[name] = val
|
||||||
|
return val
|
||||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||||
|
|
||||||
# MMA Core dataclasses (ThinkingSegment, Ticket, Track, WorkerContext, TrackMetadata)
|
# MMA Core dataclasses (ThinkingSegment, Ticket, Track, WorkerContext, TrackMetadata)
|
||||||
@@ -301,76 +306,19 @@ def __getattr__(name: str) -> Any:
|
|||||||
# tool_bias both want to import from each other via models).
|
# tool_bias both want to import from each other via models).
|
||||||
|
|
||||||
#region: UI/Editor
|
#region: UI/Editor
|
||||||
|
# TextEditorConfig + ExternalEditorConfig + EMPTY_TEXT_EDITOR_CONFIG moved
|
||||||
@dataclass
|
# to src/external_editor.py in module_taxonomy_refactor_20260627 Phase 3f.
|
||||||
class TextEditorConfig:
|
# The re-exports are LAZY via the __getattr__ below to avoid the cycle
|
||||||
name: str = ""
|
# (external_editor was previously importing them from models; the cycle
|
||||||
path: str = ""
|
# would deadlock on eager re-export).
|
||||||
diff_args: List[str] = field(default_factory=list)
|
|
||||||
|
|
||||||
def to_dict(self) -> Metadata:
|
|
||||||
"""
|
|
||||||
[C: src/personas.py:PersonaManager.save_persona, src/presets.py:PresetManager.save_preset, src/project_manager.py:save_project, src/project_manager.py:save_track_state, src/tool_presets.py:ToolPresetManager.save_bias_profile, src/tool_presets.py:ToolPresetManager.save_preset, src/workspace_manager.py:WorkspaceManager.save_profile, tests/test_bias_models.py:test_bias_profile_model, tests/test_bias_models.py:test_tool_model, tests/test_bias_models.py:test_tool_preset_extension, tests/test_context_presets_models.py:test_context_preset_serialization, tests/test_context_presets_models.py:test_file_view_preset_serialization, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_round_trip_annotations, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_serialization_with_annotations, tests/test_event_serialization.py:test_user_request_event_serialization, tests/test_external_editor.py:TestExternalEditorConfig.test_to_dict, tests/test_external_editor.py:TestTextEditorConfig.test_to_dict, tests/test_file_item_model.py:test_file_item_to_dict, tests/test_gui_events_v2.py:test_user_request_event_payload, tests/test_history_manager.py:TestHistoryManager.test_snapshot_roundtrip, tests/test_mcp_config.py:test_mcp_configuration_to_from_dict, tests/test_mcp_config.py:test_mcp_server_config_to_from_dict, tests/test_per_ticket_model.py:test_model_override_serialization, tests/test_persona_id.py:test_ticket_persona_id_serialization, tests/test_persona_models.py:test_persona_defaults, tests/test_persona_models.py:test_persona_serialization, tests/test_slice_editor_behavior.py:test_add_slice_with_annotations, tests/test_thinking_gui.py:test_thinking_segment_model_compatibility, tests/test_ticket_queue.py:test_ticket_to_dict_priority, tests/test_tiered_aggregation.py:test_persona_aggregation_strategy, tests/test_track_state_schema.py:test_track_state_to_dict, tests/test_track_state_schema.py:test_track_state_to_dict_with_none, tests/test_ui_summary_only_removal.py:test_file_item_serialization_with_flags]
|
|
||||||
"""
|
|
||||||
return {
|
|
||||||
"name": self.name,
|
|
||||||
"path": self.path,
|
|
||||||
"diff_args": self.diff_args,
|
|
||||||
}
|
|
||||||
|
|
||||||
@classmethod
|
|
||||||
def from_dict(cls, data: Metadata) -> "TextEditorConfig":
|
|
||||||
"""
|
|
||||||
[C: src/personas.py:PersonaManager.load_all, src/presets.py:PresetManager.load_all, src/project_manager.py:load_project, src/project_manager.py:load_track_state, src/tool_presets.py:ToolPresetManager.load_all_bias_profiles, src/tool_presets.py:ToolPresetManager.load_all_presets, src/workspace_manager.py:WorkspaceManager.load_all_profiles, tests/test_bias_models.py:test_bias_profile_model, tests/test_bias_models.py:test_tool_model, tests/test_bias_models.py:test_tool_preset_extension, tests/test_context_presets_models.py:test_context_preset_from_dict_legacy, tests/test_context_presets_models.py:test_context_preset_serialization, tests/test_context_presets_models.py:test_file_view_preset_serialization, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_deserialization_with_annotations, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_round_trip_annotations, tests/test_external_editor.py:TestExternalEditorConfig.test_from_dict_with_dict_editors, tests/test_external_editor.py:TestExternalEditorConfig.test_from_dict_with_string_editors, tests/test_external_editor.py:TestTextEditorConfig.test_from_dict_with_diff_args, tests/test_external_editor.py:TestTextEditorConfig.test_from_dict_without_diff_args, tests/test_file_item_model.py:test_file_item_from_dict, tests/test_file_item_model.py:test_file_item_from_dict_defaults, tests/test_history_manager.py:TestHistoryManager.test_snapshot_roundtrip, tests/test_mcp_config.py:test_mcp_configuration_to_from_dict, tests/test_mcp_config.py:test_mcp_server_config_to_from_dict, tests/test_per_ticket_model.py:test_model_override_default_on_deserialize, tests/test_per_ticket_model.py:test_model_override_deserialization, tests/test_persona_id.py:test_ticket_persona_id_deserialization, tests/test_persona_models.py:test_persona_defaults, tests/test_persona_models.py:test_persona_deserialization, tests/test_project_serialization.py:TestProjectSerialization.test_backward_compatibility_strings, tests/test_slice_editor_behavior.py:test_add_slice_with_annotations, tests/test_ticket_queue.py:test_ticket_from_dict_default_priority, tests/test_ticket_queue.py:test_ticket_from_dict_priority, tests/test_tiered_aggregation.py:test_persona_aggregation_strategy, tests/test_track_state_schema.py:test_track_state_from_dict, tests/test_track_state_schema.py:test_track_state_from_dict_empty_and_missing, tests/test_ui_summary_only_removal.py:test_file_item_serialization_with_flags]
|
|
||||||
"""
|
|
||||||
return cls(
|
|
||||||
name = data["name"],
|
|
||||||
path = data["path"],
|
|
||||||
diff_args = data.get("diff_args", []),
|
|
||||||
)
|
|
||||||
|
|
||||||
@dataclass
|
|
||||||
class ExternalEditorConfig:
|
|
||||||
editors: Dict[str, TextEditorConfig] = field(default_factory=dict)
|
|
||||||
default_editor: Optional[str] = None
|
|
||||||
|
|
||||||
def get_default(self) -> TextEditorConfig:
|
|
||||||
"""
|
|
||||||
[C: tests/test_external_editor.py:TestExternalEditorConfig.test_get_default_fallback_to_first, tests/test_external_editor.py:TestExternalEditorConfig.test_get_default_returns_configured, tests/test_external_editor.py:TestExternalEditorConfig.test_get_default_returns_none_when_empty]
|
|
||||||
"""
|
|
||||||
if self.default_editor and self.default_editor in self.editors:
|
|
||||||
return self.editors[self.default_editor]
|
|
||||||
if self.editors:
|
|
||||||
return next(iter(self.editors.values()))
|
|
||||||
return EMPTY_TEXT_EDITOR_CONFIG
|
|
||||||
|
|
||||||
def to_dict(self) -> Metadata:
|
|
||||||
"""
|
|
||||||
[C: src/personas.py:PersonaManager.save_persona, src/presets.py:PresetManager.save_preset, src/project_manager.py:save_project, src/project_manager.py:save_track_state, src/tool_presets.py:ToolPresetManager.save_bias_profile, src/tool_presets.py:ToolPresetManager.save_preset, src/workspace_manager.py:WorkspaceManager.save_profile, tests/test_bias_models.py:test_bias_profile_model, tests/test_bias_models.py:test_tool_model, tests/test_bias_models.py:test_tool_preset_extension, tests/test_context_presets_models.py:test_context_preset_serialization, tests/test_context_presets_models.py:test_file_view_preset_serialization, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_round_trip_annotations, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_serialization_with_annotations, tests/test_event_serialization.py:test_user_request_event_serialization, tests/test_external_editor.py:TestExternalEditorConfig.test_to_dict, tests/test_external_editor.py:TestTextEditorConfig.test_to_dict, tests/test_file_item_model.py:test_file_item_to_dict, tests/test_gui_events_v2.py:test_user_request_event_payload, tests/test_history_manager.py:TestHistoryManager.test_snapshot_roundtrip, tests/test_mcp_config.py:test_mcp_configuration_to_from_dict, tests/test_mcp_config.py:test_mcp_server_config_to_from_dict, tests/test_per_ticket_model.py:test_model_override_serialization, tests/test_persona_id.py:test_ticket_persona_id_serialization, tests/test_persona_models.py:test_persona_defaults, tests/test_persona_models.py:test_persona_serialization, tests/test_slice_editor_behavior.py:test_add_slice_with_annotations, tests/test_thinking_gui.py:test_thinking_segment_model_compatibility, tests/test_ticket_queue.py:test_ticket_to_dict_priority, tests/test_tiered_aggregation.py:test_persona_aggregation_strategy, tests/test_track_state_schema.py:test_track_state_to_dict, tests/test_track_state_schema.py:test_track_state_to_dict_with_none, tests/test_ui_summary_only_removal.py:test_file_item_serialization_with_flags]
|
|
||||||
"""
|
|
||||||
return {
|
|
||||||
"editors": {k: v.to_dict() for k, v in self.editors.items()},
|
|
||||||
"default_editor": self.default_editor,
|
|
||||||
}
|
|
||||||
|
|
||||||
@classmethod
|
|
||||||
def from_dict(cls, data: Metadata) -> "ExternalEditorConfig":
|
|
||||||
"""
|
|
||||||
[C: src/personas.py:PersonaManager.load_all, src/presets.py:PresetManager.load_all, src/project_manager.py:load_project, src/project_manager.py:load_track_state, src/tool_presets.py:ToolPresetManager.load_all_bias_profiles, src/tool_presets.py:ToolPresetManager.load_all_presets, src/workspace_manager.py:WorkspaceManager.load_all_profiles, tests/test_bias_models.py:test_bias_profile_model, tests/test_bias_models.py:test_tool_model, tests/test_bias_models.py:test_tool_preset_extension, tests/test_context_presets_models.py:test_context_preset_from_dict_legacy, tests/test_context_presets_models.py:test_context_preset_serialization, tests/test_context_presets_models.py:test_file_view_preset_serialization, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_deserialization_with_annotations, tests/test_custom_slices_annotations.py:test_file_item_custom_slices_round_trip_annotations, tests/test_external_editor.py:TestExternalEditorConfig.test_from_dict_with_dict_editors, tests/test_external_editor.py:TestExternalEditorConfig.test_from_dict_with_string_editors, tests/test_external_editor.py:TestTextEditorConfig.test_from_dict_with_diff_args, tests/test_external_editor.py:TestTextEditorConfig.test_from_dict_without_diff_args, tests/test_file_item_model.py:test_file_item_from_dict, tests/test_file_item_model.py:test_file_item_from_dict_defaults, tests/test_history_manager.py:TestHistoryManager.test_snapshot_roundtrip, tests/test_mcp_config.py:test_mcp_configuration_to_from_dict, tests/test_mcp_config.py:test_mcp_server_config_to_from_dict, tests/test_per_ticket_model.py:test_model_override_default_on_deserialize, tests/test_per_ticket_model.py:test_model_override_deserialization, tests/test_persona_id.py:test_ticket_persona_id_deserialization, tests/test_persona_models.py:test_persona_defaults, tests/test_persona_models.py:test_persona_deserialization, tests/test_project_serialization.py:TestProjectSerialization.test_backward_compatibility_strings, tests/test_slice_editor_behavior.py:test_add_slice_with_annotations, tests/test_ticket_queue.py:test_ticket_from_dict_default_priority, tests/test_ticket_queue.py:test_ticket_from_dict_priority, tests/test_tiered_aggregation.py:test_persona_aggregation_strategy, tests/test_track_state_schema.py:test_track_state_from_dict, tests/test_track_state_schema.py:test_track_state_from_dict_empty_and_missing, tests/test_ui_summary_only_removal.py:test_file_item_serialization_with_flags]
|
|
||||||
"""
|
|
||||||
editors = {}
|
|
||||||
for name, ed_data in data.get("editors", {}).items():
|
|
||||||
if isinstance(ed_data, dict): editors[name] = TextEditorConfig.from_dict(ed_data)
|
|
||||||
elif isinstance(ed_data, str): editors[name] = TextEditorConfig(name=name, path=ed_data)
|
|
||||||
return cls(editors=editors, default_editor=data.get("default_editor"))
|
|
||||||
|
|
||||||
|
|
||||||
EMPTY_TEXT_EDITOR_CONFIG: TextEditorConfig = TextEditorConfig()
|
|
||||||
|
|
||||||
#region: Persona
|
#region: Persona
|
||||||
# Persona dataclass moved to src/personas.py in module_taxonomy_refactor_20260627 Phase 3.4.
|
# Persona dataclass moved to src/personas.py in module_taxonomy_refactor_20260627 Phase 3.4.
|
||||||
# PersonaManager (the ops layer) is also there. Re-export at the top of this module
|
# PersonaManager (the ops layer) is also there. Re-export at the top of this module
|
||||||
# preserves backward-compat 'from src.models import Persona'.
|
# preserves backward-compat 'from src.models import Persona'.
|
||||||
|
# Persona dataclass moved to src/personas.py in module_taxonomy_refactor_20260627 Phase 3.4.
|
||||||
|
# PersonaManager (the ops layer) is also there. Re-export at the top of this module
|
||||||
|
# preserves backward-compat 'from src.models import Persona'.
|
||||||
|
|
||||||
#region: Workspace
|
#region: Workspace
|
||||||
#region: Workspace
|
#region: Workspace
|
||||||
|
|||||||
Reference in New Issue
Block a user