bpo-40246: Revert reporting of invalid string prefixes #19888

lysnikolaou · 2020-05-03T20:30:31Z

Due to backwards compatibility concerns, that keywords immediately
followed by a string without whitespace between them
(like in bg="#d00" if clear else"#fca") will fail to parse,
GH-19476 has to be reverted.

https://bugs.python.org/issue40246

Due to backwards compatibility concerns, that keywords immediately followed by a string without whitespace between them (like in `bg="#d00" if clear else"#fca"`) will fail to parse, PR19476 has to be reverted.

gvanrossum

Please also delete the NEWS item of the reverted change (Misc/NEWS.d/next/Core and Builtins/2020-04-11-17-52-03.bpo-40246.vXPze5.rst).

bedevere-bot · 2020-05-03T20:45:25Z

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.

lysnikolaou · 2020-05-03T21:00:20Z

Can't find the news file. There is no file on master with bpo-40246 in the filename according to Github and to my local clone.

hroncok · 2020-05-03T22:11:02Z

That news file has already been merged to the changelog when 3.9.0a6 was released. I think this should explicitly add a new entry about the revert.

hroncok · 2020-05-03T23:31:30Z

Misc/NEWS.d/next/Core and Builtins/2020-05-03-23-28-11.bpo-40246.c1D7x8.rst

@@ -0,0 +1 @@
+Reporting a specialised error message for invalid string prefixes, which was introduced in #19888, is being reverted due to backwards compatibility concerns for strings that immediately follow a reserved keyword without whitespace between them. Constructs like `bg="#d00" if clear else"#fca"` were failing to parse, which is not an acceptable breakage on such short notice.


19888 is this one

Whoops. Thanks for the catch!

lysnikolaou · 2020-05-03T23:38:54Z

I have made the requested changes; please review again

bedevere-bot · 2020-05-03T23:38:57Z

Thanks for making the requested changes!

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

pablogsal · 2020-05-04T00:21:21Z

Misc/NEWS.d/next/Core and Builtins/2020-05-03-23-28-11.bpo-40246.c1D7x8.rst

@@ -0,0 +1 @@
+Reporting a specialised error message for invalid string prefixes, which was introduced in GH-19476, is being reverted due to backwards compatibility concerns for strings that immediately follow a reserved keyword without whitespace between them. Constructs like `bg="#d00" if clear else"#fca"` were failing to parse, which is not an acceptable breakage on such short notice.


Please, mention the bpo number instead of the PR number (GH-19476)

Misc/NEWS.d/next/Core and Builtins/2020-05-03-23-28-11.bpo-40246.c1D7x8.rst

hroncok · 2020-05-04T10:46:24Z

I've backported this PR to Fedora's build of 3.9.0a6 and I still see:

  File "/usr/lib/python3.9/site-packages/dnf/comps.py", line 111
    return'.'.join(lcl)
         ^
SyntaxError: invalid string prefix

I will double check the patch was actually applied.

hroncok · 2020-05-04T10:51:41Z

It was a cache issue, it used an older, unpatched build. Sorry for the noise.

In fact, this PR makes the problem go away, as ecpected.

lysnikolaou · 2020-05-04T10:52:27Z

I'm not getting a SyntaxError when building this branch locally.

╰─ ./python.exe
Python 3.9.0a6+ (heads/master-dirty:64224a4727, May  3 2020, 23:23:39)
[Clang 11.0.0 (clang-1100.0.33.8)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> def f():
...     return'.'.join(lcl)
...
>>> f()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 2, in f
NameError: name 'lcl' is not defined

Misc/NEWS.d/next/Core and Builtins/2020-05-03-23-28-11.bpo-40246.c1D7x8.rst

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

lysnikolaou requested a review from pablogsal as a code owner May 3, 2020 20:30

the-knights-who-say-ni added the CLA signed label May 3, 2020

bedevere-bot added the awaiting review label May 3, 2020

lysnikolaou changed the title ~~bpo-40426: Revert reporting of invalid string prefixes~~ bpo-40246: Revert reporting of invalid string prefixes May 3, 2020

bpo-40246: Revert reporting of invalid string prefixes

1c3eeaa

Due to backwards compatibility concerns, that keywords immediately followed by a string without whitespace between them (like in `bg="#d00" if clear else"#fca"`) will fail to parse, PR19476 has to be reverted.

lysnikolaou force-pushed the revert-invalid-string-prefixes branch from 5f002af to 1c3eeaa Compare May 3, 2020 20:32

lysnikolaou added skip news and removed skip news labels May 3, 2020

gvanrossum approved these changes May 3, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels May 3, 2020

gvanrossum requested changes May 3, 2020

View reviewed changes

bedevere-bot removed the awaiting merge label May 3, 2020

bedevere-bot added the awaiting changes label May 3, 2020

gvanrossum added the skip news label May 3, 2020

This was referenced May 3, 2020

Fix a syntax typo freeipa/freeipa#4636

Closed

Fix a syntax typo leamas/ddupdate#38

Merged

Fix a syntax typo dmeranda/demjson#35

Open

Fix a syntax typo wbond/asn1crypto#181

Merged

📜🤖 Added by blurb_it.

ea8beec

lysnikolaou removed the skip news label May 3, 2020

hroncok reviewed May 3, 2020

View reviewed changes

Correct PR number in NEWS file

d117a60

bedevere-bot added awaiting change review and removed awaiting changes labels May 3, 2020

bedevere-bot requested a review from gvanrossum May 3, 2020 23:38

pablogsal reviewed May 4, 2020

View reviewed changes

Misc/NEWS.d/next/Core and Builtins/2020-05-03-23-28-11.bpo-40246.c1D7x8.rst Outdated Show resolved Hide resolved

gvanrossum approved these changes May 4, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 4, 2020

pablogsal reviewed May 4, 2020

View reviewed changes

Misc/NEWS.d/next/Core and Builtins/2020-05-03-23-28-11.bpo-40246.c1D7x8.rst Outdated Show resolved Hide resolved

Use issue number instead of PR number

9657bbd

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

pablogsal approved these changes May 4, 2020

View reviewed changes

pablogsal merged commit 846d8b2 into python:master May 4, 2020

bedevere-bot removed the awaiting merge label May 4, 2020

lysnikolaou deleted the revert-invalid-string-prefixes branch May 7, 2020 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-40246: Revert reporting of invalid string prefixes #19888

bpo-40246: Revert reporting of invalid string prefixes #19888

lysnikolaou commented May 3, 2020 •

edited

gvanrossum left a comment

bedevere-bot commented May 3, 2020

lysnikolaou commented May 3, 2020 •

edited

hroncok commented May 3, 2020

hroncok May 3, 2020

lysnikolaou May 3, 2020

lysnikolaou commented May 3, 2020

bedevere-bot commented May 3, 2020

pablogsal May 4, 2020

hroncok commented May 4, 2020 •

edited

hroncok commented May 4, 2020

lysnikolaou commented May 4, 2020 •

edited

		@@ -0,0 +1 @@
		Reporting a specialised error message for invalid string prefixes, which was introduced in #19888, is being reverted due to backwards compatibility concerns for strings that immediately follow a reserved keyword without whitespace between them. Constructs like `bg="#d00" if clear else"#fca"` were failing to parse, which is not an acceptable breakage on such short notice.

bpo-40246: Revert reporting of invalid string prefixes #19888

bpo-40246: Revert reporting of invalid string prefixes #19888

Conversation

lysnikolaou commented May 3, 2020 • edited

gvanrossum left a comment

Choose a reason for hiding this comment

bedevere-bot commented May 3, 2020

lysnikolaou commented May 3, 2020 • edited

hroncok commented May 3, 2020

hroncok May 3, 2020

Choose a reason for hiding this comment

lysnikolaou May 3, 2020

Choose a reason for hiding this comment

lysnikolaou commented May 3, 2020

bedevere-bot commented May 3, 2020

pablogsal May 4, 2020

Choose a reason for hiding this comment

hroncok commented May 4, 2020 • edited

hroncok commented May 4, 2020

lysnikolaou commented May 4, 2020 • edited

lysnikolaou commented May 3, 2020 •

edited

lysnikolaou commented May 3, 2020 •

edited

hroncok commented May 4, 2020 •

edited

lysnikolaou commented May 4, 2020 •

edited