feat: add workspace functionality with manager and UI integration by Xcesius · Pull Request #3 · Xcesius/Diablo2TextEditor

Xcesius · 2025-08-24T21:27:47Z

No description provided.

Copilot

Pull Request Overview

This PR introduces comprehensive workspace functionality with a tabbed editor interface, replacing the single-view approach with a multi-file workspace system. The implementation includes workspace management for saving/loading application states, a file panel for quick navigation, and enhanced UI layout with tabbed editing.

Key changes:

Added workspace save/load/delete functionality with persistent JSON storage
Replaced single table view with tabbed editor interface supporting multiple open files
Added workspace file panel with drag-and-drop support for file management

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
workspaces.json	Configuration file storing saved workspace states with window geometry and view settings
workspace_manager.py	Core workspace management class providing JSON-based persistence for workspace snapshots
ui.py	Major UI overhaul replacing single table with tabbed interface, workspace panel, and multi-file support
main_v2.py	Added Qt logging filters to suppress debug noise during development
custom_widgets.py	Enhanced table views with better editor ownership tracking and debug capabilities
config.json	Updated configuration with new workspace settings and file tracking

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-24T21:29:56Z

+        "column_widths": [
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100,
+          100
+        ],


The column_widths array contains 105 repeated values of 100. This repetitive data suggests the array could be replaced with a more compact representation like {count: 105, width: 100} or dynamically generated default values to improve maintainability.

Suggested change

"column_widths": [

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100,

100

],

"column_widths": {

"count": 105,

"width": 100

},

Copilot · 2025-08-24T21:29:57Z

+        except Exception:
+            pass


The broad exception handling with bare 'except Exception:' and 'pass' suppresses all errors including programming errors. Consider logging the exception or handling specific exception types that are expected during normal operation.

Suggested change

except Exception:

pass

except Exception as e:

logging.exception("Failed to set _d2te_index property on editor in ComboBoxDelegate.createEditor")

Copilot · 2025-08-24T21:29:57Z

+        # Load the first selected into the editor
+        self._load_file_path(paths[0])



The method silently loads only the first file when multiple files are selected (line 1119), which may be unexpected behavior for users. Consider either loading all selected files in tabs or clearly indicating in the UI that only the first file will be opened.

Suggested change

# Load the first selected into the editor

self._load_file_path(paths[0])

# Load all selected files into the editor (in tabs)

for file_path in paths:

self._load_file_path(file_path)

Copilot · 2025-08-24T21:29:57Z

+
+    def on_tab_bar_clicked(self, index: int):
+        # Try to finalize edits in the current tab before switching
+        self._close_all_active_editors()
+
+    def _close_all_active_editors(self):
+        try:
+            from PyQt6.QtWidgets import QApplication
+            # Prefer closing the currently focused inline editor only; this avoids cross-view commits
+            fw = QApplication.focusWidget()
+            if fw is not None:
+                # Find owning QTableView and top-level editor widget
+                from PyQt6.QtWidgets import QTableView, QComboBox, QLineEdit, QSpinBox, QDoubleSpinBox, QDateEdit, QTimeEdit, QDateTimeEdit
+                editor_types = (QComboBox, QLineEdit, QSpinBox, QDoubleSpinBox, QDateEdit, QTimeEdit, QDateTimeEdit)
+                # Ascend from focus widget to find top-level editor
+                ed = fw
+                top_editor = None
+                depth = 0
+                while ed is not None and depth < 10:  # Increased depth
+                    if isinstance(ed, editor_types):
+                        top_editor = ed
+                    ed = ed.parent()
+                    depth += 1
+                # If focus was inside editable combobox, use the combobox itself
+                if isinstance(top_editor, QLineEdit) and isinstance(top_editor.parent(), QComboBox):
+                    top_editor = top_editor.parent()
+
+                # More thorough search for owning view - check all known table views
+                owner_view = None
+                all_views = list(self._tabs.values())
+                if hasattr(self, 'tableView') and self.tableView is not None:
+                    all_views.append(self.tableView)
+


The method imports QApplication inside the function and performs complex widget hierarchy traversal on every call. Consider moving the import to the module level and caching widget references to improve performance, especially since this method may be called frequently during tab switching.

Suggested change

def on_tab_bar_clicked(self, index: int):

# Try to finalize edits in the current tab before switching

self._close_all_active_editors()

def _close_all_active_editors(self):

try:

from PyQt6.QtWidgets import QApplication

# Prefer closing the currently focused inline editor only; this avoids cross-view commits

fw = QApplication.focusWidget()

if fw is not None:

# Find owning QTableView and top-level editor widget

from PyQt6.QtWidgets import QTableView, QComboBox, QLineEdit, QSpinBox, QDoubleSpinBox, QDateEdit, QTimeEdit, QDateTimeEdit

editor_types = (QComboBox, QLineEdit, QSpinBox, QDoubleSpinBox, QDateEdit, QTimeEdit, QDateTimeEdit)

# Ascend from focus widget to find top-level editor

ed = fw

top_editor = None

depth = 0

while ed is not None and depth < 10: # Increased depth

if isinstance(ed, editor_types):

top_editor = ed

ed = ed.parent()

depth += 1

# If focus was inside editable combobox, use the combobox itself

if isinstance(top_editor, QLineEdit) and isinstance(top_editor.parent(), QComboBox):

top_editor = top_editor.parent()

# More thorough search for owning view - check all known table views

owner_view = None

all_views = list(self._tabs.values())

if hasattr(self, 'tableView') and self.tableView is not None:

all_views.append(self.tableView)

# Update cached views after tab change

self._update_all_views_cache()

def on_tab_bar_clicked(self, index: int):

# Try to finalize edits in the current tab before switching

self._close_all_active_editors()

def _update_all_views_cache(self):

# Cache all known table views for performance

self._all_views_cache = list(self._tabs.values())

if hasattr(self, 'tableView') and self.tableView is not None:

if self.tableView not in self._all_views_cache:

self._all_views_cache.append(self.tableView)

def _close_all_active_editors(self):

try:

# Prefer closing the currently focused inline editor only; this avoids cross-view commits

fw = QApplication.focusWidget()

if fw is not None:

# Find owning QTableView and top-level editor widget

# Ascend from focus widget to find top-level editor

ed = fw

top_editor = None

depth = 0

while ed is not None and depth < 10: # Increased depth

if isinstance(ed, EDITOR_WIDGET_TYPES):

top_editor = ed

ed = ed.parent()

depth += 1

# If focus was inside editable combobox, use the combobox itself

if isinstance(top_editor, QLineEdit) and isinstance(top_editor.parent(), QComboBox):

top_editor = top_editor.parent()

# More thorough search for owning view - check all known table views

owner_view = None

all_views = getattr(self, '_all_views_cache', list(self._tabs.values()))

Copilot · 2025-08-24T21:29:58Z

+            debug_on = False
+            try:
+                debug_on = self.config_manager.get_setting("debug_mode_enabled", False)
+            except Exception:
+                debug_on = False


The nested try-catch blocks for retrieving debug settings create unnecessary complexity. Consider extracting this debug flag retrieval into a helper method since it's used multiple times throughout the class.

Suggested change

debug_on = False

try:

debug_on = self.config_manager.get_setting("debug_mode_enabled", False)

except Exception:

debug_on = False

debug_on = self._is_debug_enabled()

Copilot · 2025-08-24T21:29:58Z

+        except Exception:
+            view = None
+            parent_to_use = parent


The broad exception handling suppresses all exceptions including potential programming errors. Consider handling specific exceptions like AttributeError that might occur during normal operation, and logging unexpected exceptions for debugging.

Suggested change

except Exception:

view = None

parent_to_use = parent

except (AttributeError, TypeError):

view = None

parent_to_use = parent

except Exception as e:

print(f"[ERROR] Unexpected exception in createEditor parent determination: {e}")

view = None

parent_to_use = parent

feat: add workspace functionality with manager and UI integration

2cd1ba8

Copilot AI review requested due to automatic review settings August 24, 2025 21:27

Copilot AI reviewed Aug 24, 2025

View reviewed changes

Xcesius closed this Aug 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add workspace functionality with manager and UI integration#3

feat: add workspace functionality with manager and UI integration#3
Xcesius wants to merge 1 commit intomainfrom
feat/workspace-v1

Xcesius commented Aug 24, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 24, 2025

Uh oh!

Copilot AI Aug 24, 2025

Uh oh!

Copilot AI Aug 24, 2025

Uh oh!

Copilot AI Aug 24, 2025

Uh oh!

Copilot AI Aug 24, 2025

Uh oh!

Copilot AI Aug 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        "column_widths": [
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-,
-        ],
+        "column_widths": {
+          "count": 105,
+          "width": 100
+        },

-        except Exception:
-            pass
+        except Exception as e:
+            logging.exception("Failed to set _d2te_index property on editor in ComboBoxDelegate.createEditor")

		# Load the first selected into the editor
		self._load_file_path(paths[0])

-        # Load the first selected into the editor
-        self._load_file_path(paths[0])
+        # Load all selected files into the editor (in tabs)
+        for file_path in paths:
+            self._load_file_path(file_path)

Conversation

Xcesius commented Aug 24, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants