Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow NUL character in f-strings #7378

Merged
merged 1 commit into from
Sep 14, 2023
Merged

Allow NUL character in f-strings #7378

merged 1 commit into from
Sep 14, 2023

Conversation

dhruvmanila
Copy link
Member

Summary

This PR fixes the bug to allow NUL (\0) character inside f-strings.

Test Plan

Add test case with NUL character inside f-string.

@dhruvmanila
Copy link
Member Author

dhruvmanila commented Sep 14, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@dhruvmanila dhruvmanila mentioned this pull request Sep 14, 2023
Merged
dhruvmanila
dhruvmanila commented Sep 14, 2023
View reviewed changes
crates/ruff_python_parser/src/lexer.rs
Comment on lines +555 to +558
// The condition is to differentiate between the `NUL` (`\0`) character
// in the source code and the one returned by `self.cursor.first()` when
// we reach the end of the source code.
EOF_CHAR if self.cursor.is_eof() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

We could move this in the catch-all branch but that would mean that the check will be performed for almost every character.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm yeah. That's why using self.cursor.first() is "dangerous" and I prefer to use bump wherever I can. Can you do a quick double check if any other code needs to check for self.cursor.is_eof()

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's the only place where it seems necessary.

@dhruvmanila dhruvmanila added parser Related to the parser python312 Related to Python 3.12 labels Sep 14, 2023
MichaReiser
MichaReiser approved these changes Sep 14, 2023
View reviewed changes
crates/ruff_python_parser/src/lexer.rs
Comment on lines +555 to +558
// The condition is to differentiate between the `NUL` (`\0`) character
// in the source code and the one returned by `self.cursor.first()` when
// we reach the end of the source code.
EOF_CHAR if self.cursor.is_eof() => {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm yeah. That's why using self.cursor.first() is "dangerous" and I prefer to use bump wherever I can. Can you do a quick double check if any other code needs to check for self.cursor.is_eof()

@dhruvmanila dhruvmanila merged commit 5d0063a into dhruv/pep-701 Sep 14, 2023
@dhruvmanila dhruvmanila deleted the dhruv/fstring-nul branch September 14, 2023 10:58
@dhruvmanila dhruvmanila mentioned this pull request Sep 15, 2023
Closed
@dhruvmanila dhruvmanila linked an issue Sep 15, 2023 that may be closed by this pull request
Add support for PEP 701 in the lexer #7042
Closed
dhruvmanila added a commit that referenced this pull request Sep 18, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
93d2e0c 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
This was referenced Sep 18, 2023
Merged
Merged
Merged
Merged
Merged
Merged
Merged
dhruvmanila added a commit that referenced this pull request Sep 19, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
ea6f32c 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
@dhruvmanila dhruvmanila mentioned this pull request Sep 19, 2023
Merged
dhruvmanila added a commit that referenced this pull request Sep 20, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
afa2669 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
dhruvmanila added a commit that referenced this pull request Sep 22, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
777a217 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
dhruvmanila added a commit that referenced this pull request Sep 22, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
d01dbf5 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
This was referenced Sep 22, 2023
Merged
Merged
Merged
dhruvmanila added a commit that referenced this pull request Sep 22, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
1e0b304 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
@dhruvmanila dhruvmanila mentioned this pull request Sep 22, 2023
Merged
dhruvmanila added a commit that referenced this pull request Sep 26, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
ceb127c 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
@dhruvmanila dhruvmanila mentioned this pull request Sep 26, 2023
Merged
dhruvmanila added a commit that referenced this pull request Sep 27, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
6997940 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
dhruvmanila added a commit that referenced this pull request Sep 28, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
e6f36b9 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
@dhruvmanila dhruvmanila mentioned this pull request Sep 28, 2023
Merged
dhruvmanila added a commit that referenced this pull request Sep 29, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
3f81cb1 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
dhruvmanila added a commit that referenced this pull request Sep 29, 2023
@dhruvmanila
Allow NUL character in f-strings (#7378)
846948f 
## Summary

This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings.

## Test Plan

Add test case with `NUL` character inside f-string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees
No one assigned
Labels
parser Related to the parser python312 Related to Python 3.12
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for PEP 701 in the lexer
2 participants
@dhruvmanila @MichaReiser