[cmd] Undeprecate deferredProxy #7417
Conversation
This changes the way deferred proxy is implemented to not use the deprecated ProxyCommand constructor. This function serves a good purpose that should be kept IMO. The constructor was confusing but this is just good syntactic sugar over `defer(() -> supplier.get().asProxy())`. Signed-off-by: Jade Turner <[email protected]>
|
This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR. |
| return ProxyCommand(std::move(supplier)).ToPtr(); | ||
| return Defer( | ||
| [supplier = std::move(supplier)]() mutable { | ||
| return ProxyCommand{std::move(supplier())}.ToPtr(); |
There was a problem hiding this comment.
I couldn't make this work using the AsProxy decorator
There was a problem hiding this comment.
Since the return of supplier() is already an rvalue, the move isn't needed.
The following should work:
return supplier().AsProxy()(You can also pop a ToPtr there in between, but I think it's unneeded)
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh I missed that we're dealing with a Command* and not a CommandPtr.
Never mind.
There was a problem hiding this comment.
I don't think there's a non-owning version of AsProxy(), so yes, I think this does have to be via the constructor. Maybe just add a comment explaining that so we don't forget?
There was a problem hiding this comment.
The std::move on line 80 is still unnecessary as the return value of supplier() is Command*
Not applicable to Python. |
This changes the way deferred proxy is implemented to not use the deprecated ProxyCommand constructor.
This function serves a good purpose that should be kept IMO. The constructor was confusing but this is just good syntactic sugar over
defer(() -> supplier.get().asProxy()).