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.

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

Fix end location of compound statements #4327

Merged
merged 9 commits into from Dec 13, 2022
Merged

Fix end location of compound statements #4327

merged 9 commits into from Dec 13, 2022
+94 −67

Conversation

harupy
Copy link
Contributor

@harupy harupy commented Dec 11, 2022 •
edited

For example, FunctionDef currently ends at where DEDENT occurs:

def f1():
    pass



def f2():
^ here
    pass

but it should end at the final statement in the function body:

def f1():
    pass
       ^ should end here



def f2():
    pass

@harupy harupy changed the title Fix end location of Fix end location of nodes that contain body Dec 11, 2022
@harupy harupy marked this pull request as ready for review Dec 11, 2022
@harupy harupy requested a review from DimitrisJim Dec 12, 2022
@harupy harupy changed the title Fix end location of nodes that contain body Fix end location of compound statements Dec 12, 2022
harupy
harupy commented Dec 12, 2022
View changes
compiler/parser/python.lalrpop Outdated Show resolved Hide resolved
@harupy harupy force-pushed the fix-end-location-body branch from 6abd795 to 7e315c1 Compare Dec 12, 2022
DimitrisJim
DimitrisJim reviewed Dec 12, 2022
View changes
Copy link
Member

@DimitrisJim DimitrisJim left a comment

thanks! I've left a couple of comments on this

compiler/parser/python.lalrpop Outdated Show resolved Hide resolved
compiler/parser/python.lalrpop Outdated Show resolved Hide resolved
compiler/parser/python.lalrpop Outdated Show resolved Hide resolved
@harupy
Copy link
Contributor Author

harupy commented Dec 12, 2022

@DimitrisJim Thanks for the comments! Applied your suggestions.

@harupy harupy requested a review from DimitrisJim Dec 12, 2022
DimitrisJim
DimitrisJim approved these changes Dec 13, 2022
View changes
Copy link
Member

@DimitrisJim DimitrisJim left a comment

looks good to me, thanks!

@DimitrisJim DimitrisJim merged commit 8d879a5 into RustPython:main Dec 13, 2022
13 checks passed
@harupy harupy deleted the fix-end-location-body branch Dec 13, 2022
@harupy
Copy link
Contributor Author

harupy commented Dec 13, 2022

cc @charliermarsh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DimitrisJim DimitrisJim

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@harupy @DimitrisJim