Skip to content

gh-135148: Correctly handle f/t strings with comments and debug expressions #135198

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 6, 2025


// Process each character of input
Copy link
Member Author

Choose a reason for hiding this comment

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

@lysnikolaou please read this carefully. I am mostly sure its correct but it really needs some extra eyes

Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

Yeah, I was thinking something very similar as a solution, too! I found one problem, but it looks good otherwise.

Comment on lines 189 to 197
if (i > 0 && tok_mode->last_expr_buffer[i-1] == ch &&
i > 1 && tok_mode->last_expr_buffer[i-2] == ch) {
// Found triple quote - copy all three quotes
result[j++] = ch;
result[j++] = ch;
result[j++] = ch;
i += 2; // Skip the other two quotes
continue;
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is gonna work as intended. Let's say we have a triple-quoted string. Then, I think the following sequence is going to happen:

  1. ch == '"', !in_string, so we're doing in_string = 1; string_quote = ch;
  2. ch == '"', in_string, ch == string_quote, but the inside if fails because we're only on the second quote, so we're setting in_string = 0 back
  3. ch == '"', !in_string and we're setting in_string = 1 again.

For example the following does not work correctly:

>>> f'{ # some comment goes here
... """hello"""=}'
' \n"""hello"""""\'hello\''

Same goes for the construct above where we're doing the hash detection. Example that fails there:

>>> f'{""" # no c
... """ # c
... =}'
'""" # no c\n""" # c\n=\' # no c\\n\''

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch sir!

I have pushed a version that addresses this and simplifies stuff a bit

@bedevere-app
Copy link

bedevere-app bot commented Jun 6, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants