black icon indicating copy to clipboard operation
black copied to clipboard

Long string in return annotation does not get split if there's parameters

Open jakkdl opened this issue 2 years ago • 3 comments

Describe the bug This was first reported in #2699, and thought resolved in #2990, except that only solved the specific case of the function having no parameters.

To Reproduce Run the test return_annotation_brackets_string:

https://github.com/psf/black/blob/3a2d76c7bcf39e852f3b379b76537d7847ed4225/tests/data/preview/return_annotation_brackets_string.py#L1-L23

The second function should be formatted exactly the same as the first one.

jakkdl avatar Oct 06 '23 13:10 jakkdl

hey i am new to open source contribution coming from hacktoberfest , can you help me/assign me this issue so that i could gain some experience

GOWTHAM2K1 avatar Oct 07 '23 07:10 GOWTHAM2K1

Learning to navigate the different transforms made to the code and what got changed were was a bit tricky for me, but with enough breakpoint debugging and added prints I managed to get a decent handle on it . For this issue you've got the StringSplitter class in src/black/trans.py which is called from transform_line in src/black/linegen.py, which is otherwise responsible for splitting standalone strings. And then there's left_hand_split also in linegen.py which is responsible for splitting function bodies, and in #3916 I added should_split_funcdef_with_rhs to improve return type parsing. One idea I had would be to expand the logic in StringSplitter to not just trigger on bare strings, but also the case where it's the return value (i.e. following an RARROW). There might definitely be better ways of going about it though.

I'd suggest you just get started on trying to modify shit and see if you manage to make any type of progress, and if so just open a PR where reviewers can give more concrete feedback if your solution has any problems. I'm not going to work on this and suspect it's relatively low priority, so you probably don't need to worry about duplicating work or anything like that.

Make sure you read https://github.com/psf/black#contributing and that also has a link to the Python discord which among other things has a #black channel, you can maybe find more help there if needed. You can also read other PRs that have been merged recently and try to understand them.

Good luck!

jakkdl avatar Oct 07 '23 12:10 jakkdl

Thank you very much for your guidance.

On Sat, 7 Oct 2023, 5:33 pm John Litborn, @.***> wrote:

Learning to navigate the different transforms made to the code and what got changed were was a bit tricky for me, but with enough breakpoint debugging and added prints I managed to get a decent handle on it . For this issue you've got the StringSplitter class in src/black/trans.py which is called from transform_line in src/black/linegen.py, which is otherwise responsible for splitting standalone strings. And then there's left_hand_split also in linegen.py which is responsible for splitting function bodies, and in #3916 https://github.com/psf/black/pull/3916 I added should_split_funcdef_with_rhs to improve return type parsing. One idea I had would be to expand the logic in StringSplitter to not just trigger on bare strings, but also the case where it's the return value (i.e. following an RARROW). There might definitely be better ways of going about it though.

I'd suggest you just get started on trying to modify shit and see if you manage to make any type of progress, and if so just open a PR where reviewers can give more concrete feedback if your solution has any problems. I'm not going to work on this and suspect it's relatively low priority, so you probably don't need to worry about duplicating work or anything like that.

Make sure you read https://github.com/psf/black#contributing and that also has a link to the Python discord which among other things has a #black channel, you can maybe find more help there if needed.

Good luck!

— Reply to this email directly, view it on GitHub https://github.com/psf/black/issues/3926#issuecomment-1751694173, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXH2OE3HLEJDMPMEECBTPDDX6FAKPAVCNFSM6AAAAAA5V3XFO2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJRGY4TIMJXGM . You are receiving this because you commented.Message ID: @.***>

GOWTHAM2K1 avatar Oct 07 '23 13:10 GOWTHAM2K1