◐ Shell
reader mode source ↗
Skip to content

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

Merged
pablogsal merged 4 commits into
python:masterfrom
lysnikolaou:revert-invalid-string-prefixes
May 4, 2020
Merged

bpo-40246: Revert reporting of invalid string prefixes#19888
pablogsal merged 4 commits into
python:masterfrom
lysnikolaou:revert-invalid-string-prefixes

Conversation

@lysnikolaou

@lysnikolaou lysnikolaou commented May 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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

@lysnikolaou lysnikolaou requested a review from pablogsal as a code owner May 3, 2020 20:30
@lysnikolaou lysnikolaou changed the title bpo-40426: Revert reporting of invalid string prefixes May 3, 2020
@lysnikolaou
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 lysnikolaou force-pushed the revert-invalid-string-prefixes branch from 5f002af to 1c3eeaa Compare May 3, 2020 20:32
gvanrossum
gvanrossum requested changes May 3, 2020
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

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

bedevere-bot commented May 3, 2020

Copy link
Copy Markdown

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

lysnikolaou commented May 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

hroncok commented May 3, 2020

Copy link
Copy Markdown
Contributor

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.

This was referenced May 3, 2020
Closed
Merged
Open
Merged
@lysnikolaou

lysnikolaou commented May 3, 2020

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented May 3, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

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

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

hroncok commented May 4, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

hroncok commented May 4, 2020

Copy link
Copy Markdown
Contributor

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

lysnikolaou commented May 4, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

@lysnikolaou @pablogsal
Use issue number instead of PR number …
9657bbd 
Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
@pablogsal pablogsal merged commit 846d8b2 into python:master May 4, 2020
@lysnikolaou lysnikolaou deleted the revert-invalid-string-prefixes branch May 7, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

@pablogsal pablogsal pablogsal approved these changes

+1 more reviewer

@hroncok hroncok hroncok left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@lysnikolaou @bedevere-bot @hroncok @gvanrossum @pablogsal @the-knights-who-say-ni