Skip to content

Cripts: make URL::Query copyable via deep-copy of _state#13269

Open
serrislew wants to merge 4 commits into
apache:masterfrom
serrislew:cripts_query
Open

Cripts: make URL::Query copyable via deep-copy of _state#13269
serrislew wants to merge 4 commits into
apache:masterfrom
serrislew:cripts_query

Conversation

@serrislew

Copy link
Copy Markdown
Contributor

Url::Query holds its parsed-parameter scratch space in a std::unique_ptr<State>, which implicitly deletes the copy ctor and copy-assign. This PR defines the copy operations explicitly to deep-copy *_state, and = defaults the moves so they aren't suppressed by the user-declared copies.

Copilot AI review requested due to automatic review settings June 15, 2026 19:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes cripts::Url::Query copyable by explicitly defining copy ctor/copy-assign to deep-copy the internal std::unique_ptr<State> scratch space, and re-enables move operations that would otherwise be suppressed. It adds coverage in Cripts gold tests and updates a Cripts test snippet to exercise the new copy operations.

Changes:

  • Add explicit cripts::Url::Query copy ctor / copy-assign (deep-copy _state) and default move operations.
  • Add a new Cripts gold-test plugin and test run validating copy + mutation behavior of Url::Query.
  • Update an existing Cripts test snippet to use the new copy ctor.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
include/cripts/Urls.hpp Implements explicit copy/move operations for Url::Query by deep-copying _state.
src/cripts/tests/query_test.cc Exercises Url::Query copy construction and logs before/after mutation.
tests/gold_tests/cripts/files/query_copy.cript New Cripts plugin used by gold tests to validate copy/copy-assign behavior.
tests/gold_tests/cripts/cripts.test.py Adds an origin transaction, remap, and a new curl-based gold test run for query copy behavior.

Comment thread include/cripts/Urls.hpp
Comment thread tests/gold_tests/cripts/files/query_copy.cript Outdated
Comment thread src/cripts/tests/query_test.cc Outdated
Copilot AI review requested due to automatic review settings June 15, 2026 22:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread tests/gold_tests/cripts/cripts.test.py Outdated
Comment thread src/cripts/tests/query_test.cc
@cmcfarlen cmcfarlen requested a review from zwoop June 15, 2026 22:14
@cmcfarlen cmcfarlen added this to the 11.0.0 milestone Jun 15, 2026
Copilot AI review requested due to automatic review settings June 16, 2026 17:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread tests/gold_tests/cripts/cripts.test.py Outdated
Comment thread include/cripts/Urls.hpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants