Skip to content

fix: restore coin control Qt build#46

Open
thepastaclaw wants to merge 1 commit into
PastaPastaPasta:feat/coin-control-shift-selectfrom
thepastaclaw:fix-7260-ci-coin-control
Open

fix: restore coin control Qt build#46
thepastaclaw wants to merge 1 commit into
PastaPastaPasta:feat/coin-control-shift-selectfrom
thepastaclaw:fix-7260-ci-coin-control

Conversation

@thepastaclaw

@thepastaclaw thepastaclaw commented May 29, 2026

Copy link
Copy Markdown

Restore coin-control Qt build

Summary

  • Move the coin-control checkbox hit-test helper into
    CoinControlTreeWidget.
  • Keep the same checkbox-column and check-indicator hit testing while allowing
    access to protected QTreeWidget::indexFromItem.
  • Leave the conflict-prediction workflow alone; the relevant upstream workflow
    fix is already on develop.

Validation

  • Reproduced the original Qt build failure against the pre-fix code path:
    protected QTreeWidget::indexFromItem access from a free helper.
  • Ran a targeted -fsyntax-only compile of
    src/qt/coincontroltreewidget.cpp after the fix with local Qt5 headers:
    passed.
  • Ran the pre-PR code-review gate for
    6b5a5553bf6fb3e60acb734ec86d3c2def06166c..fcb84c34345c76c1b6325f0e3bf3e13a69a03b23:
    ship.

Summary by CodeRabbit

  • Refactor
    • Internal code improvements to the coin control tree widget implementation with no user-facing changes.

Review Change Stack

QTreeWidget::indexFromItem is protected, so the free helper failed to
compile under Qt 5. Move the helper into CoinControlTreeWidget so it
inherits access; behavior (column, validity, and check-indicator hit
test) is unchanged.
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d9a1930-c3ec-4aaa-9d33-0b8636573386

📥 Commits

Reviewing files that changed from the base of the PR and between 6b5a555 and fcb84c3.

📒 Files selected for processing (2)
  • src/qt/coincontroltreewidget.cpp
  • src/qt/coincontroltreewidget.h

📝 Walkthrough

Walkthrough

The PR refactors a checkbox hit-test helper from a static function to a private member function of CoinControlTreeWidget. The static helper's tree-widget parameter is eliminated by using the instance context (this), and the call site in mouseReleaseEvent is updated to use the new signature.

Changes

Checkbox hit-test helper refactoring

Layer / File(s) Summary
Convert checkbox hit-test to member function
src/qt/coincontroltreewidget.h, src/qt/coincontroltreewidget.cpp
The static isCheckboxClick(QTreeWidget*, QTreeWidgetItem*, QPoint) helper is moved to a private member method isCheckboxClick(QTreeWidgetItem*, const QPoint&) const that derives context from this, and mouseReleaseEvent is updated to call it with the new signature.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A helper hops from static ground,
Now nested in the widget's crown,
No pointer passed, just this will do—
Checkbox clicks, now cleaner too!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: restore coin control Qt build' directly relates to the main change: moving the checkbox hit-test helper into the CoinControlTreeWidget class to resolve a Qt build failure caused by accessing a protected method from a free function.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@thepastaclaw

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw

thepastaclaw commented Jun 15, 2026

Copy link
Copy Markdown
Author

✅ Review complete (commit fcb84c3)

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Small, well-scoped Qt build fix promoting the static isCheckboxClick helper into a private const member of CoinControlTreeWidget so the call to the protected QTreeWidget::indexFromItem compiles. The hit-test logic and call-site behavior are unchanged; only the receiver changes from an explicit tree parameter to this. Both independent agents found no issues; verification against the diff confirms a faithful mechanical move with no correctness, security, or consensus implications.

Posted as a COMMENT review because GitHub does not allow PastaClaw to approve their own PR.

@thepastaclaw thepastaclaw marked this pull request as ready for review June 16, 2026 00:56
@thepastaclaw

Copy link
Copy Markdown
Author

CI follow-up on the current red predict_conflicts check: the predictor itself found no conflicts (conflictPrs: []) and checked out the synthetic merge ref successfully. The job only failed when mshick/add-pr-comment tried to post the "No Merge Conflicts Detected" status and GitHub returned Resource not accessible by integration, so this is a workflow token/permissions issue rather than a PR conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant