Extracted widget code into own class - #27
Conversation
Reviewer's GuideRefactors the Python node UI into a dedicated GtpyPythonNodeWidget class, exposes small helper APIs on GtpyPythonNode to support the widget, and slightly modernizes the CMake configuration and sources list. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
The license is not used anymore, since we deleted GTlab.cmake
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
GtpyPythonNodeWidget::create, returning an emptyunique_ptrwhen the node is not aGtpyPythonNodemay violateregisterWidgetFactoryexpectations; consider asserting the type or providing a minimal fallback widget instead of returning null. - In
gtpn_pythonnodewidget.cppheaders likegtpy_contextmanager.h,gtpy_gilscope.h, andgtpy_tempdir.hare included but not used in this file; they can likely be removed to reduce compile-time dependencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `GtpyPythonNodeWidget::create`, returning an empty `unique_ptr` when the node is not a `GtpyPythonNode` may violate `registerWidgetFactory` expectations; consider asserting the type or providing a minimal fallback widget instead of returning null.
- In `gtpn_pythonnodewidget.cpp` headers like `gtpy_contextmanager.h`, `gtpy_gilscope.h`, and `gtpy_tempdir.h` are included but not used in this file; they can likely be removed to reduce compile-time dependencies.
## Individual Comments
### Comment 1
<location path="src/gui/gtpn_pythonnodewidget.cpp" line_range="74-83" />
<code_context>
+ }
+}
+
+void
+GtpyPythonNodeWidget::updateTimePassed(int progress)
+{
+ if (progress != 100)
+ {
+ m_button->setIcon(gt::gui::icon::processRunningIcon(progress));
+ }
+ else
+ {
+ m_button->setIcon(gt::gui::icon::python());
+ }
+}
+
+void
+GtpyPythonNodeWidget::showCustomContextMenu(const QPoint& pos)
+{
+ if (!m_textEdit)
+ {
+ return;
+ }
+
+ QMenu* menu = m_textEdit->createStandardContextMenu();
+ QAction* customAction = new QAction(QStringLiteral("Assistant"), menu);
+
+ connect(customAction, &QAction::triggered,
+ this, &GtpyPythonNodeWidget::insertAssistantPrompt);
+
+ menu->addSeparator();
+ menu->addAction(customAction);
+ menu->exec(m_textEdit->mapToGlobal(pos));
+ delete menu;
+}
+
+void
+GtpyPythonNodeWidget::insertAssistantPrompt()
+{
+ if (!m_textEdit)
</code_context>
<issue_to_address>
**issue (bug_risk):** The Assistant menu installation is guarded by a different capability than the prompt invocation, which may lead to a no-op action.
`openScriptEditor()` installs the context menu based on `getSharedFunction("Assistant", "prompt")`, but `insertAssistantPrompt()` calls `getSharedFunction("Assistant", "open_prompt_python")`. If only one of these is available, the "Assistant" menu will appear but do nothing when clicked. Consider basing the menu installation on the availability of `open_prompt_python`, or using a single consistent capability key for both the check and the call.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Thanks, good catches. I’ll remove the unused includes. For |
Description
Extract the Python node UI into a dedicated widget class and adjust the Python node to delegate UI concerns to it.
Enhancements:
Closes #26
How Has This Been Tested?
Checklist:
EXPORT. Non-interface functions are NOT exported.Summary by Sourcery
Extract the Python node UI into a dedicated widget class and delegate UI responsibilities from the node to this widget.
Enhancements:
Build: