-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix string_processing crash for one-item tuple string literals #4917
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The test cases don't seem to be picked up by our tests runner. Please put them in Additionally, can you add a changelog entry to Thanks for working on this! |
|
Hi @cobaltt7 Thanks for the review! I’ve moved the test cases to tests/data/cases/string_trailing_comma.py and combined all three related cases into a single file. Added the # flags: --unstable comment at the top so it runs correctly under preview mode. Added a changelog entry in CHANGES.md. Ran Black on the codebase for consistent formatting. All changes are now pushed to the string-processing branch. Please let me know if there’s anything else needed! Thanks again for the guidance. |
|
I'm inclined to believe that this PR is AI-generated. While using AI in itself isn't forbidden, we do expect you to look over what the AI did, that you've reviewed the code it writes, and that submitted PRs are up to our code standards and in line with the project expectations and requirements. Unfortunately, your contribution does not meet this bar. Please review the docs on writing test cases; it's still not correct. https://black.readthedocs.io/en/stable/contributing/the_basics.html#testing |
|
This is my first contribution to Black, and I understand now that while the issue and fix approach were valid, my tests and changelog entry don’t yet meet the project’s conventions. I’ll review the testing documentation more carefully, rework the tests to follow the expected structure, and correct the changelog entry accordingly.
I appreciate you pointing this out and will update the PR once it’s aligned with Black’s standards.
…________________________________
From: cobalt ***@***.***>
Sent: Thursday, December 25, 2025 5:30 AM
To: psf/black ***@***.***>
Cc: Harish Nesrekar ***@***.***>; Author ***@***.***>
Subject: Re: [psf/black] Fix string_processing crash for one-item tuple string literals (PR #4917)
[https://avatars.githubusercontent.com/u/61329810?s=20&v=4]cobaltt7 left a comment (psf/black#4917)<#4917 (comment)>
I'm inclined to believe that this PR is AI-generated. While using AI in itself isn't forbidden, we do expect you to look over what the AI did, that you've reviewed the code it writes, and that submitted PRs are up to our code standards and in line with the project expectations and requirements. Unfortunately, your contribution does not meet this bar.
Please review the docs on writing test cases; it's still not correct. https://black.readthedocs.io/en/stable/contributing/the_basics.html#testing
Please also correct the changelog entry.
—
Reply to this email directly, view it on GitHub<#4917 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BTMIVW6YKGEROEDKH4CDJQL4DMSLHAVCNFSM6AAAAACP6UWSDKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMOJQGYZDMOBRHE>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Description
Fix crash in string-processing preview for unassigned one-item tuples of string literals with a trailing comma.
"A very long string that exceeds line length and has a trailing comma",because it tried to split the string but did not wrap it in parentheses, producing code that is not AST-equivalent.
Cause:
_prefer_paren_wrap_match()decides whether to wrap a literal in parentheses. Previously,syms.testlist(one-item tuples) was missing, so parentheses weren’t added.Fix: Add
syms.testlistto_prefer_paren_wrap_match():This is a minimal, safe fix that preserves AST semantics.
Added regression test:
tests/data/preview/string_processing_trailing_comma.pyVerified with:
Checklist - did you ...
--previewstyle?CHANGES.mdif necessary?