Skip to content

python driver: preserve trailing quotes in agtype string values#2425

Merged
jrgemignani merged 2 commits into
apache:masterfrom
SAY-5:fix/python-string-strip-quotes
Jun 2, 2026
Merged

python driver: preserve trailing quotes in agtype string values#2425
jrgemignani merged 2 commits into
apache:masterfrom
SAY-5:fix/python-string-strip-quotes

Conversation

@SAY-5
Copy link
Copy Markdown
Contributor

@SAY-5 SAY-5 commented May 7, 2026

Fixes #2418.

ResultVisitor.visitStringValue() and visitPair() in drivers/python/age/builder.py use str.strip('"') to remove the surrounding delimiters from agtype STRING tokens. str.strip() removes all matching characters from both ends, so when a property value or object key starts or ends with an escaped quote the parser drops the data character along with the delimiter:

Input agtype Expected Before this PR
"foo \"bar\"" foo \"bar\" foo \"bar\
"\"leading" \"leading leading
"trailing\"" trailing\" trailing\

The Agtype grammar (drivers/Agtype.g4) guarantees STRING : '"' (ESC | SAFECODEPOINT)* '"', so the token always has exactly one " delimiter on each side. Slicing with [1:-1] removes them precisely without touching the body.

Test added: test_string_value_preserves_inner_quotes covers leading/trailing/embedded escaped quotes plus the visitPair() path for object keys, and exercises the empty-string "" edge case.

@jrgemignani jrgemignani requested a review from Copilot May 13, 2026 14:37
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jrgemignani jrgemignani requested a review from Copilot May 13, 2026 15:17
@jrgemignani
Copy link
Copy Markdown
Contributor

@SAY-5 Please rebase :)

Copy link
Copy Markdown

Copilot AI left a comment

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 2 out of 2 changed files in this pull request and generated 4 comments.

Comment thread drivers/python/age/builder.py Outdated
Comment thread drivers/python/age/builder.py Outdated
Comment thread drivers/python/age/builder.py Outdated
Comment thread drivers/python/test_agtypes.py Outdated
@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented May 13, 2026

Rebased onto master.

@jrgemignani
Copy link
Copy Markdown
Contributor

@SAY-5 Can you address Copilot's comments above? Please address then in each comment :) It makes it easier to verify.

@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented May 22, 2026

@jrgemignani Addressed all four Copilot comments in e265425: centralized the trim logic into _stripStringDelimiters and reused it in both visitStringValue/visitPair, removed the stray space before the comma, and reformatted the test docstring. Replied on each inline thread.

SAY-5 added 2 commits May 24, 2026 19:26
str.strip('"') in visitStringValue() and visitPair() removes every '"'
on either side of the token, not just the outer delimiters, so a value
ending in an escaped quote (e.g. '"foo \"bar\""') loses its trailing
backslash-escaped '"' character.  The Agtype grammar guarantees STRING
tokens always carry exactly one delimiter on each side, so slice with
[1:-1] to strip them precisely.

Fixes apache#2418

Signed-off-by: SAY-5 <saiasish.cnp@gmail.com>
@SAY-5 SAY-5 force-pushed the fix/python-string-strip-quotes branch from e265425 to 1c816a1 Compare May 25, 2026 02:27
@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented May 25, 2026

Rebased on current master.

Copy link
Copy Markdown

Copilot AI left a comment

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 2 out of 2 changed files in this pull request and generated no new comments.

@jrgemignani jrgemignani merged commit 5a254d6 into apache:master Jun 2, 2026
6 checks passed
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.

Python driver: Double quotes at the end of string values are wrongfully removed

3 participants