add "Accept All" to Peer Dialog Receive File context menu#725
Merged
Conversation
lidaobing
marked this pull request as draft
July 14, 2026 05:30
Reviewer's guide (collapsed on small PRs)Reviewer's GuideExpose a new window-level "accept_all" GAction for receiving files, wire the receive-file button and peer dialog context menu to this action, and rename the button from "Accept" to "Accept All" while reusing the existing bulk-accept logic for both automatic and user-invoked acceptance flows. Sequence diagram for the new accept_all file reception actionsequenceDiagram
actor User
participant AcceptAllButton
participant GAction_accept_all
participant DialogPeer
User->>AcceptAllButton: click
AcceptAllButton->>GAction_accept_all: activate
GAction_accept_all->>DialogPeer: onAcceptAll
Flow diagram for entry points into DialogPeer.onAcceptAllflowchart TD
A[ShowInfoEnclosure] -->|receiving > 0| B[onAcceptAll]
C[Accept All button] --> D[win.accept_all]
D --> B
E[RecvTreePopup context menu] --> F[win.accept_all]
F --> B
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider updating the
onAcceptcallback signature to use the specific GAction/GVariant pointer types (e.g.,GSimpleAction*andGVariant*) instead ofvoid*to improve type safety and readability when used as aGActionCallback. - In
ShowInfoEnclosure, call the static handler asDialogPeer::onAccept(nullptr, nullptr, dlgpr);instead ofdlgpr->onAccept(...)to make it clearer this is a static action callback rather than an instance method.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider updating the `onAccept` callback signature to use the specific GAction/GVariant pointer types (e.g., `GSimpleAction*` and `GVariant*`) instead of `void*` to improve type safety and readability when used as a `GActionCallback`.
- In `ShowInfoEnclosure`, call the static handler as `DialogPeer::onAccept(nullptr, nullptr, dlgpr);` instead of `dlgpr->onAccept(...)` to make it clearer this is a static action callback rather than an instance method.
## Individual Comments
### Comment 1
<location path="src/iptux/DialogPeer.cpp" line_range="849" />
<code_context>
if (receiving > 0)
- dlgpr->onAcceptButtonClicked(dlgpr);
+ dlgpr->onAccept(nullptr, nullptr, dlgpr);
}
/**
</code_context>
<issue_to_address>
**suggestion:** Avoid calling the GAction callback directly with synthetic nullptr arguments.
This ties `ShowInfoEnclosure` to the exact `onAccept` callback signature and assumes the `GSimpleAction` and `GVariant` parameters are never needed, which is brittle and may break if `onAccept` starts using them. Prefer either triggering the action via the action group (e.g. `g_action_group_activate_action` for "accept") or factoring the core accept logic into a separate helper that both `onAccept` and `ShowInfoEnclosure` call, with `onAccept` just adapting the GAction callback signature.
Suggested implementation:
```cpp
}
if (receiving > 0)
g_action_group_activate_action(G_ACTION_GROUP(dlgpr->window), "accept", nullptr);
}
```
This change assumes that:
1. `dlgpr->window` is the same object used earlier with `g_action_map_disable_actions(G_ACTION_MAP(self.window), ...)` and implements `GActionGroup`.
2. The "accept" action is already registered on that window/action group and wired to `onAccept`.
If `DialogPeer` exposes the action group via another member or accessor (rather than `window`), adjust `G_ACTION_GROUP(dlgpr->window)` accordingly. If the accept action expects a parameter, replace the final `nullptr` with an appropriate `GVariant` value.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| if (receiving > 0) | ||
| dlgpr->onAcceptButtonClicked(dlgpr); | ||
| dlgpr->onAccept(nullptr, nullptr, dlgpr); |
There was a problem hiding this comment.
suggestion: Avoid calling the GAction callback directly with synthetic nullptr arguments.
This ties ShowInfoEnclosure to the exact onAccept callback signature and assumes the GSimpleAction and GVariant parameters are never needed, which is brittle and may break if onAccept starts using them. Prefer either triggering the action via the action group (e.g. g_action_group_activate_action for "accept") or factoring the core accept logic into a separate helper that both onAccept and ShowInfoEnclosure call, with onAccept just adapting the GAction callback signature.
Suggested implementation:
}
if (receiving > 0)
g_action_group_activate_action(G_ACTION_GROUP(dlgpr->window), "accept", nullptr);
}
This change assumes that:
dlgpr->windowis the same object used earlier withg_action_map_disable_actions(G_ACTION_MAP(self.window), ...)and implementsGActionGroup.- The "accept" action is already registered on that window/action group and wired to
onAccept.
If DialogPeer exposes the action group via another member or accessor (rather than window), adjust G_ACTION_GROUP(dlgpr->window) accordingly. If the accept action expects a parameter, replace the final nullptr with an appropriate GVariant value.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #725 +/- ##
=======================================
Coverage 51.58% 51.59%
=======================================
Files 64 64
Lines 8865 8866 +1
=======================================
+ Hits 4573 4574 +1
Misses 4292 4292 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an “Accept” action to the peer dialog’s receive-file context menu and wires the existing “Accept” receive button to a new win.accept GAction, so file reception control is accessible via both button and popup menu.
Changes:
- Add Accept item to the
peer-recv-popupmenu (win.accept). - Register a new window action
acceptand route the Accept button throughGtkActionable. - Update receive-tree selection logic to toggle action sensitivity for
accept/refuse.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/iptux/resources/gtk/menus.ui |
Adds “Accept” to the receive-file popup menu via win.accept. |
src/iptux/DialogPeer.h |
Replaces the old accept-click handler declaration with the new action callback signature. |
src/iptux/DialogPeer.cpp |
Registers win.accept, hooks the Accept button to it, and updates action sensitivity toggling. |
|
❌ The last analysis has failed. |
lidaobing
marked this pull request as ready for review
July 15, 2026 04:09
lidaobing
changed the title
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider adding a thin wrapper (e.g.
acceptAll()method onDialogPeer) and having both the action callback andShowInfoEnclosurecall that, rather than calling theonAcceptAllaction callback directly withnullptrarguments. - The new
onAcceptAllsignature usingvoid*, void*parameters is hard to follow; it would be clearer and safer to use the actual GTK types (e.g.GSimpleAction*,GVariant*) or explicitly document why generic pointers are needed formakeActionEntry. - Now that the button is wired to
accept_all, double-check whether the button label should explicitly sayAccept Allor justAcceptbased on what the handler actually does (accepting all files vs. only selected ones) to avoid user confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a thin wrapper (e.g. `acceptAll()` method on `DialogPeer`) and having both the action callback and `ShowInfoEnclosure` call that, rather than calling the `onAcceptAll` action callback directly with `nullptr` arguments.
- The new `onAcceptAll` signature using `void*, void*` parameters is hard to follow; it would be clearer and safer to use the actual GTK types (e.g. `GSimpleAction*`, `GVariant*`) or explicitly document why generic pointers are needed for `makeActionEntry`.
- Now that the button is wired to `accept_all`, double-check whether the button label should explicitly say `Accept All` or just `Accept` based on what the handler actually does (accepting all files vs. only selected ones) to avoid user confusion.
## Individual Comments
### Comment 1
<location path="src/iptux/DialogPeer.cpp" line_range="933" />
<code_context>
- */
-void DialogPeer::onAcceptButtonClicked(DialogPeer* self) {
+
+void DialogPeer::onAcceptAll(void*, void*, DialogPeer* self) {
GtkWidget* widget;
GtkTreeModel* model;
</code_context>
<issue_to_address>
**suggestion:** Align the callback signature and unused parameters with the patterns used by `onRefuse`/`onRefuseAll` for consistency and clarity.
`onAcceptAll` currently takes two anonymous `void*` parameters and a `DialogPeer*`, whereas other action callbacks (e.g., `onRefuse`/`onRefuseAll`) use `DialogPeer& self` and name the first two parameters as `GSimpleAction*` / `GVariant*`. Please update this to use a `DialogPeer&` if possible, use the concrete GTK/GIO types for the first two parameters, or explicitly mark them unused (casts or `[[maybe_unused]]`) to match the existing pattern and avoid warnings.
Suggested implementation:
```cpp
void DialogPeer::onAcceptAll(GSimpleAction* action, GVariant* parameter, DialogPeer& self) {
(void)action;
(void)parameter;
```
To fully align with the existing pattern, you will also need to:
1. Update the corresponding declaration in `DialogPeer.hpp` (or the relevant header) to match the new signature:
- From: `static void onAcceptAll(void*, void*, DialogPeer* self);`
- To: `static void onAcceptAll(GSimpleAction* action, GVariant* parameter, DialogPeer& self);`
2. Adjust any places where `onAcceptAll` is connected as a callback (e.g., `g_signal_connect` or `g_simple_action_new_stateful` callbacks) to pass a `DialogPeer&` (or ensure the callback trampoline matches the new signature), following the same pattern as `onRefuse`/`onRefuseAll`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| */ | ||
| void DialogPeer::onAcceptButtonClicked(DialogPeer* self) { | ||
|
|
||
| void DialogPeer::onAcceptAll(void*, void*, DialogPeer* self) { |
There was a problem hiding this comment.
suggestion: Align the callback signature and unused parameters with the patterns used by onRefuse/onRefuseAll for consistency and clarity.
onAcceptAll currently takes two anonymous void* parameters and a DialogPeer*, whereas other action callbacks (e.g., onRefuse/onRefuseAll) use DialogPeer& self and name the first two parameters as GSimpleAction* / GVariant*. Please update this to use a DialogPeer& if possible, use the concrete GTK/GIO types for the first two parameters, or explicitly mark them unused (casts or [[maybe_unused]]) to match the existing pattern and avoid warnings.
Suggested implementation:
void DialogPeer::onAcceptAll(GSimpleAction* action, GVariant* parameter, DialogPeer& self) {
(void)action;
(void)parameter;
To fully align with the existing pattern, you will also need to:
- Update the corresponding declaration in
DialogPeer.hpp(or the relevant header) to match the new signature:- From:
static void onAcceptAll(void*, void*, DialogPeer* self); - To:
static void onAcceptAll(GSimpleAction* action, GVariant* parameter, DialogPeer& self);
- From:
- Adjust any places where
onAcceptAllis connected as a callback (e.g.,g_signal_connectorg_simple_action_new_statefulcallbacks) to pass aDialogPeer&(or ensure the callback trampoline matches the new signature), following the same pattern asonRefuse/onRefuseAll.
Comment on lines
+933
to
936
| void DialogPeer::onAcceptAll(void*, void*, DialogPeer* self) { | ||
| GtkWidget* widget; | ||
| GtkTreeModel* model; | ||
| GtkTreeIter iter; |
Comment on lines
75
to
81
| static void onRecvTreeSelectionChanged(DialogPeer& self, GtkTreeSelection*); | ||
| static void onAcceptButtonClicked(DialogPeer* self); | ||
| static void ShowInfoEnclosure(DialogPeer* dlgpr); | ||
| static bool UpdataEnclosureRcvUI(DialogPeer* dlgpr); | ||
| static gint RcvTreePopup(GtkWidget*, GdkEvent* event, DialogPeer* self); | ||
| static void onAcceptAll(void*, void*, DialogPeer* self); | ||
| static void onRefuse(void*, void*, DialogPeer& self); | ||
| static void onRefuseAll(void*, void*, DialogPeer& self); |
Comment on lines
+341
to
+344
| <item> | ||
| <attribute name="label" translatable="yes">Accept All</attribute> | ||
| <attribute name="action">win.accept_all</attribute> | ||
| </item> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Enhancements: