bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt #17582

lysnikolaou · 2019-12-12T20:21:34Z

When parsing an "elif" node, lineno and col_offset of the node now point to the "elif" keyword and not to its condition, making it consistent with the "if" node.

https://bugs.python.org/issue39031

Automerge-Triggered-By: @pablogsal


        bpo-39031: Include elif keyword when producing lineno/col-offset info…

… for if_stmt

pablogsal · 2019-12-12T20:59:32Z

Hi there 😉 Can you add a NEWS entry?

pablogsal

Can you also add a elif case to the exec_test list and regenerate the test data (check the end of the file for instructions)?

bedevere-bot · 2019-12-12T21:03:43Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.


        📜🤖 Added by blurb_it.

Misc/NEWS.d/next/Core and Builtins/2019-12-12-21-05-43.bpo-39031.imlCYZ.rst


        Update News file based on feedback

Co-Authored-By: Pablo Galindo <Pablogsal@gmail.com>


        Include an elif case in exec_test

lysnikolaou · 2019-12-12T21:20:27Z

Thanks a lot for the review @pablogsal! I have made the requested changes; please review again.

bedevere-bot · 2019-12-12T21:20:30Z

Thanks for making the requested changes!

@pablogsal: please review the changes made to this pull request.

pablogsal

Thanks for the patch @lysnikolaou! I will backport this to 3.8 and 3.7 once the CI finish.

miss-islington · 2019-12-12T21:38:40Z

@lysnikolaou: Status check is done, and it's a failure ❌ .

miss-islington · 2019-12-12T21:40:19Z

@lysnikolaou: Status check is done, and it's a success ✅ .

bedevere-bot · 2019-12-12T21:40:38Z

GH-17583 is a backport of this pull request to the 3.8 branch.

miss-islington · 2019-12-12T21:40:39Z

Sorry, @lysnikolaou, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 025a602af7ee284d8db6955c26016f3f27d35536 3.7


        [3.7] bpo-39031: Include elif keyword when producing lineno/col-offse…

@pablogsal

…t info for if_stmt (pythonGH-17582) When parsing an "elif" node, lineno and col_offset of the node now point to the "elif" keyword and not to its condition, making it consistent with the "if" node. https://bugs.python.org/issue39031 Automerge-Triggered-By: @pablogsal. (cherry picked from commit 025a602) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>


        [3.7] bpo-39031: Include elif keyword when producing lineno/col-offse…

@pablogsal

…t info for if_stmt (pythonGH-17582) When parsing an "elif" node, lineno and col_offset of the node now point to the "elif" keyword and not to its condition, making it consistent with the "if" node. https://bugs.python.org/issue39031 Automerge-Triggered-By: @pablogsal. (cherry picked from commit 025a602) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

bedevere-bot · 2019-12-12T21:51:50Z

GH-17584 is a backport of this pull request to the 3.7 branch.

miss-islington · 2019-12-13T00:56:28Z

Thanks @lysnikolaou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington · 2019-12-13T00:56:29Z

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

miss-islington · 2019-12-13T01:59:52Z

Thanks @lysnikolaou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington · 2019-12-13T01:59:55Z

Sorry @lysnikolaou, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 025a602af7ee284d8db6955c26016f3f27d35536 3.8

miss-islington · 2019-12-13T14:03:23Z

Thanks @lysnikolaou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

bedevere-bot · 2019-12-13T14:03:48Z

GH-17589 is a backport of this pull request to the 3.8 branch.


        bpo-39031: Include elif keyword when producing lineno/col-offset info…

@pablogsal

… for if_stmt (pythonGH-17582) When parsing an "elif" node, lineno and col_offset of the node now point to the "elif" keyword and not to its condition, making it consistent with the "if" node. https://bugs.python.org/issue39031 Automerge-Triggered-By: @pablogsal (cherry picked from commit 025a602) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>


        [3.7] bpo-39031: Include elif keyword when producing lineno/col-offse…

@pablogsal

…t info for if_stmt (GH-17582) (#17584) When parsing an "elif" node, lineno and col_offset of the node now point to the "elif" keyword and not to its condition, making it consistent with the "if" node. https://bugs.python.org/issue39031 Automerge-Triggered-By: @pablogsal. (cherry picked from commit 025a602) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>


        bpo-39031: Include elif keyword when producing lineno/col-offset info…

@pablogsal

… for if_stmt (GH-17582) (GH-17589) When parsing an "elif" node, lineno and col_offset of the node now point to the "elif" keyword and not to its condition, making it consistent with the "if" node. https://bugs.python.org/issue39031 Automerge-Triggered-By: @pablogsal (cherry picked from commit 025a602) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

lysnikolaou · 2019-12-13T17:15:39Z

@pablogsal I just found out that I kind of messed this up and one more change is needed. What is the right course of action here? Open a new PR for the additional change?

pablogsal · 2019-12-13T17:28:29Z

Open a new PR for the additional change?

Yes, also explain in the issue what is missing

bpo-39031: Include elif keyword when producing lineno/col-offset info…

Loading status checks…

7320207

… for if_stmt

the-knights-who-say-ni added the CLA signed label Dec 12, 2019

bedevere-bot added the awaiting review label Dec 12, 2019

lysnikolaou mentioned this pull request Dec 12, 2019

Group line and column metadata needs special handling #107

Closed

pablogsal self-requested a review Dec 12, 2019

pablogsal self-assigned this Dec 12, 2019

pablogsal requested changes Dec 12, 2019

View changes

bedevere-bot added awaiting changes and removed awaiting review labels Dec 12, 2019

📜🤖 Added by blurb_it.

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

5f58a6d

pablogsal reviewed Dec 12, 2019

View changes

Misc/NEWS.d/next/Core and Builtins/2019-12-12-21-05-43.bpo-39031.imlCYZ.rst Outdated Show resolved Hide resolved

pablogsal added needs backport to 3.7 needs backport to 3.8 labels Dec 12, 2019

lysnikolaou and others added 2 commits Dec 12, 2019

Update News file based on feedback

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

1adf59d

Co-Authored-By: Pablo Galindo <Pablogsal@gmail.com>

Include an elif case in exec_test

Loading status checks…

4fc6d4f

bedevere-bot added awaiting change review and removed awaiting changes labels Dec 12, 2019

bedevere-bot requested a review from pablogsal Dec 12, 2019

pablogsal approved these changes Dec 12, 2019

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Dec 12, 2019

pablogsal added the 🤖 automerge label Dec 12, 2019

bedevere-bot removed the needs backport to 3.8 label Dec 12, 2019

miss-islington assigned miss-islington and unassigned pablogsal Dec 12, 2019

bedevere-bot removed the needs backport to 3.7 label Dec 12, 2019

lysnikolaou mentioned this pull request Dec 12, 2019

[3.7] bpo-39031: Include elif keyword when producing lineno/col-offse… #17585

Closed

lysnikolaou deleted the lysnikolaou:ast-elif branch Dec 12, 2019

sthagen mentioned this pull request Dec 12, 2019

bpo-39031: Include elif keyword when producing lineno/col-offset info… #95

Merged

pablogsal added the needs backport to 3.8 label Dec 13, 2019

pablogsal added needs backport to 3.8 and removed needs backport to 3.8 labels Dec 13, 2019

bedevere-bot removed the needs backport to 3.8 label Dec 13, 2019

lysnikolaou mentioned this pull request Dec 14, 2019

bpo-39031: Fix elif start column offset when there is an else following #17596

Merged

python/cpython

bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt #17582

bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt #17582

lysnikolaou commented Dec 12, 2019 •

edited by miss-islington

This comment has been minimized.

pablogsal commented Dec 12, 2019

pablogsal left a comment •

edited

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

This comment has been minimized.

lysnikolaou commented Dec 12, 2019

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

pablogsal left a comment

This comment has been minimized.

miss-islington commented Dec 12, 2019

This comment has been minimized.

miss-islington commented Dec 12, 2019

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

This comment has been minimized.

miss-islington commented Dec 12, 2019

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

bedevere-bot commented Dec 13, 2019

This comment has been minimized.

lysnikolaou commented Dec 13, 2019

This comment has been minimized.

pablogsal commented Dec 13, 2019

Sponsor python/cpython

python/cpython

Join GitHub today

bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt #17582

bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt #17582

Conversation

lysnikolaou commented Dec 12, 2019 • edited by miss-islington

This comment has been minimized.

pablogsal commented Dec 12, 2019

pablogsal left a comment • edited

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

This comment has been minimized.

lysnikolaou commented Dec 12, 2019

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

pablogsal left a comment

This comment has been minimized.

miss-islington commented Dec 12, 2019

This comment has been minimized.

miss-islington commented Dec 12, 2019

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

This comment has been minimized.

miss-islington commented Dec 12, 2019

This comment has been minimized.

bedevere-bot commented Dec 12, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

miss-islington commented Dec 13, 2019

This comment has been minimized.

bedevere-bot commented Dec 13, 2019

This comment has been minimized.

lysnikolaou commented Dec 13, 2019

This comment has been minimized.

pablogsal commented Dec 13, 2019

lysnikolaou commented Dec 12, 2019 •

edited by miss-islington

pablogsal left a comment •

edited