Conversation
Added logging to help the user see behind the scenes
There was a problem hiding this comment.
Pull Request Overview
This PR adds a toggleable logging window to the PlexPlaylistMaker GUI application, allowing users to monitor application progress and rate-limiting behavior in real-time. The changes enhance user visibility into background operations and simplify troubleshooting.
Key changes include:
- Implementation of a live log window with GUI controls for show/hide/clear operations
- Addition of a custom logging handler that filters connection errors and feeds logs to the GUI
- Enhanced logging output for playlist creation with detailed statistics on matched/unmatched items
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Updated documentation to describe the new in-app log window feature and simplified Letterboxd title parsing explanation |
| PlexPlaylistMakerGUI.py | Added QueueHandler class, log window UI components, and keyboard shortcuts for toggling connection error logging |
| PlexPlaylistMakerController.py | Enhanced logging output with detailed statistics and changed some log levels from INFO to DEBUG for less verbose output |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def show_log_window(self): | ||
| if self.log_window and tk.Toplevel.winfo_exists(self.log_window): | ||
| return | ||
| self.log_window = tk.Toplevel(self) |
There was a problem hiding this comment.
The method call tk.Toplevel.winfo_exists(self.log_window) is incorrect. The winfo_exists() method should be called on the instance, not the class. It should be self.log_window.winfo_exists().
| self.log_window = tk.Toplevel(self) | |
| def toggle_log_window(self): | |
| if self.log_window and self.log_window.winfo_exists(): | |
| self.hide_log_window() | |
| else: | |
| self.show_log_window() | |
| def show_log_window(self): | |
| if self.log_window and self.log_window.winfo_exists(): | |
| return | |
| self.log_window = tk.Toplevel(self) |
| def show_log_window(self): | ||
| if self.log_window and tk.Toplevel.winfo_exists(self.log_window): | ||
| return | ||
| self.log_window = tk.Toplevel(self) |
There was a problem hiding this comment.
The method call tk.Toplevel.winfo_exists(self.log_window) is incorrect. The winfo_exists() method should be called on the instance, not the class. It should be self.log_window.winfo_exists().
| self.log_window = tk.Toplevel(self) | |
| def toggle_log_window(self): | |
| if self.log_window and self.log_window.winfo_exists(): | |
| self.hide_log_window() | |
| else: | |
| self.show_log_window() | |
| def show_log_window(self): | |
| if self.log_window and self.log_window.winfo_exists(): | |
| return | |
| self.log_window = tk.Toplevel(self) |
| def hide_log_window(self): | ||
| if self.log_window and tk.Toplevel.winfo_exists(self.log_window): | ||
| self.stop_log_polling() | ||
| self.log_window.destroy() |
There was a problem hiding this comment.
The method call tk.Toplevel.winfo_exists(self.log_window) is incorrect. The winfo_exists() method should be called on the instance, not the class. It should be self.log_window.winfo_exists().
| self.log_window.destroy() | |
| if self.log_window and self.log_window.winfo_exists(): | |
| self.stop_log_polling() | |
| self.log_window.destroy() |
| self.queue_handler.suppress_connection_errors = not self.queue_handler.suppress_connection_errors | ||
| state = 'ON' if not self.queue_handler.suppress_connection_errors else 'OFF' | ||
| # Inject an informational line so user knows the state changed | ||
| self.log_queue.put(f"[LOG FILTER] Connection error messages now {('visible' if state=='ON' else 'hidden')}.") |
There was a problem hiding this comment.
The string comparison state=='ON' is fragile since state is defined as 'ON' or 'OFF' based on a boolean condition. Consider using the boolean directly: 'visible' if not self.queue_handler.suppress_connection_errors else 'hidden'.
| self.log_queue.put(f"[LOG FILTER] Connection error messages now {('visible' if state=='ON' else 'hidden')}.") | |
| self.queue_handler.suppress_connection_errors = not self.queue_handler.suppress_connection_errors | |
| # Inject an informational line so user knows the state changed | |
| self.log_queue.put(f"[LOG FILTER] Connection error messages now {'visible' if not self.queue_handler.suppress_connection_errors else 'hidden'}.") |
Added logging to help the user see behind the scenes