Conversation
Resolves NameError encountered during runtime when opening chat view.
Error log:
File 'app/view/chat_interface.py', line 119, in __init__
self.send_btn = PrimaryPushButton('发送', self)
^^^^^^^^^^^^^^^^^
NameError: name 'PrimaryPushButton' is not defined
Fix:
Import PrimaryPushButton from qfluentwidgets in app/view/chat_interface.py.
Refactored ChatView.addMessage to use proper nested layouts: 1. Introduced top-level QHBoxLayout for the message row. 2. Added missing Avatar placeholder (IconWidget). 3. Separated NameLabel and MessageBubble into a vertical column to prevent overlap. 4. Applied distinctive styling for self/other messages (colors, borders). 5. Fixed alignment issues (Left for others, Right for self).
Summary of ChangesHello @Detritalw, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for a new desktop QQ client application, focusing on a modern user interface and robust backend integration. It provides a complete framework for real-time messaging, contact management, and user customization, built upon PyQt6 and QFluentWidgets. The changes encompass everything from API communication and local data storage to a responsive and feature-rich chat interface, marking the initial release of the application's core components. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a substantial amount of code, effectively building out the entire initial structure of the Fluent QQ application. The changes include the main application window, chat and settings interfaces, various UI components, an API client, and configuration management. While the overall structure is well-organized and utilizes modern Python and PyQt6 practices, there are several critical issues and areas for improvement regarding security, performance, and code quality that should be addressed. Key issues include a hardcoded token in a configuration file, a critical bug due to a call to a non-existent method, performance bottlenecks from synchronous network calls in the UI thread, and several code quality improvements such as replacing print statements with a proper logger and avoiding bad practices like bare except clauses.
| if not ref_msg: | ||
| # Try remote | ||
| try: | ||
| res = client.get_msg(reply_ref_id) |
There was a problem hiding this comment.
The method client.get_msg() is called here, but it is not defined in the NapCatClient class in app/common/api_client.py. This will cause a runtime AttributeError. You need to either implement the get_msg method in the API client or use an existing method to fetch a single message.
| "api_url": "http://127.0.0.1:3000", | ||
| "language": "zh_CN", | ||
| "theme": "Light", | ||
| "token": "shenmitoken", |
There was a problem hiding this comment.
Hardcoding secrets like API tokens directly into version-controlled files is a major security risk. This token is now exposed in the repository's history. It should be removed immediately. Secrets should be managed through environment variables or a local configuration file that is included in .gitignore.
| except: | ||
| pass |
There was a problem hiding this comment.
Using a bare except: pass is dangerous as it silently ignores all exceptions, including system-exiting ones like SystemExit or KeyboardInterrupt. This can hide bugs and make debugging extremely difficult. At a minimum, you should catch Exception and log the error.
| except: | |
| pass | |
| except Exception as e: | |
| print(f"[AvatarLoader] Failed to load image from {self.url}: {e}") |
| try: | ||
| conn = sqlite3.connect(self.db_path) | ||
| # Enable memory mapping for faster access | ||
| conn.execute("PRAGMA mmap_size = 30000000000") |
There was a problem hiding this comment.
The PRAGMA mmap_size is set to 30GB, which is an extremely large value and could cause issues on systems with less memory. Furthermore, this pragma is executed every time a database operation is performed, which is redundant. It should be set only once when the database connection is established. Consider reducing the mmap_size to a more reasonable value (e.g., 256MB) and setting it only in the _init_db method after the connection is created.
| chat_interface = self.window().findChild(QFrame, "chatInterface") | ||
| if chat_interface and chat_interface.self_id: | ||
| self.current_self_id = str(chat_interface.self_id) |
There was a problem hiding this comment.
Accessing parent components using self.window().findChild(...) creates tight coupling between components and makes the code harder to maintain and test. A better approach is to pass necessary information (like self_id) down to child widgets through their constructors or dedicated methods. This promotes component independence and follows the principle of dependency injection.
| except Exception as e: | ||
| print(f"[Client] API Error: {e}") | ||
| return None |
There was a problem hiding this comment.
Catching a broad Exception can hide bugs and make debugging more difficult. It's better to catch more specific exceptions that you expect to handle, such as requests.exceptions.RequestException for network errors or json.JSONDecodeError for parsing issues. This makes the error handling more precise and robust.
| except Exception as e: | |
| print(f"[Client] API Error: {e}") | |
| return None | |
| except requests.exceptions.RequestException as e: | |
| print(f"[Client] API Network Error: {e}") | |
| return None | |
| except json.JSONDecodeError as e: | |
| print(f"[Client] API JSON Decode Error: {e}") | |
| return None |
| def _load(): | ||
| try: | ||
| response = requests.get(url, timeout=5) | ||
| if response.status_code == 200: | ||
| pixmap = QPixmap() | ||
| pixmap.loadFromData(response.content) | ||
| # Update UI in main thread (not strictly safe here but ImageLabel usually handles pixmap setting well, | ||
| # but for safety we should use signals if this was complex. | ||
| # For simplicity in this context, we'll assume direct setPixmap works or needs invokeMethod. | ||
| # Actually, PySide/PyQt UI updates MUST be in main thread. | ||
| # Let's use a simple QTimer or custom signal approach if needed, | ||
| # but for now let's just use a simple synchronous load in thread with signal | ||
| # Wait, we can't emit signal from thread without defining it. | ||
| pass | ||
| except Exception as e: | ||
| print(f"[AvatarWidget] Error loading {url}: {e}") |
There was a problem hiding this comment.
This block contains an old, incorrect, and commented-out implementation for loading an image. Since the correct implementation using AvatarLoader is already in place, this dead code should be removed to improve clarity and maintainability.
| setTheme(theme) | ||
| ThemeManager.apply_theme_patches(theme) | ||
|
|
||
| from app.view.main_window import MainWindow |
There was a problem hiding this comment.
For better code organization and to adhere to PEP 8 guidelines, it's recommended to move all imports to the top of the file. Local imports, like this one for MainWindow, can sometimes be justified (e.g., to avoid circular dependencies or for optional features), but in this case, moving it to the top would improve readability and consistency.
Move `from app.view.main_window import MainWindow` to the top of the file with other imports.
app/view/components/chat_bubble.py
Outdated
|
|
||
| r_layout.addStretch(1) | ||
| layout.addWidget(self.reaction_container) | ||
| print("DEBUG: reaction_container added to layout") |
There was a problem hiding this comment.
This print statement appears to be for debugging purposes. It should be removed from the production code. For diagnostics, consider using the logging module, which allows for configurable log levels and outputs.
| self.api_url = config.get("api_url").rstrip('/') | ||
| self.ws_url = config.get("ws_url") | ||
| self.token = config.get("token") | ||
| print(f"[Client] Config refreshed: API={self.api_url}, WS={self.ws_url}, Token={'***' if self.token else 'None'}") |
There was a problem hiding this comment.
Throughout the application, print() is used for logging. For a more robust and configurable logging solution, it's highly recommended to use Python's built-in logging module. This allows you to control log levels (e.g., DEBUG, INFO, ERROR), direct output to files, and easily enable or disable logging in production builds.
1 participant
No description provided.