bpo-35133: Fix mistakes when concatenate string literals on different lines. #10284

serhiy-storchaka · 2018-11-01T12:40:34Z

Two kind of mistakes:

Missed space. After concatenating there is no space between words.
Missed comma. Causes unintentional concatenating in a list of strings.

https://bugs.python.org/issue35133

… lines. Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings.

ericvsmith

I have a few comments, the others look good. Although this whole change just points out a pitfall of having max. line length constraints: errors like the ones you're fixing are pretty common, and hard to spot.

Lib/idlelib/idle_test/htest.py

ericvsmith · 2018-11-01T12:54:39Z

Lib/test/test_email/test__header_value_parser.py

@@ -2154,11 +2154,11 @@ def test_get_group_one_invalid(self):

    def test_get_group_missing_final_semicol(self):
        group = self._test_get_x(parser.get_group,
-            ('Monty Python:"Fred A. Bear" <dinsdale@example.com>,'
+            ('Monty Python:"Fred A. Bear" <dinsdale@example.com>, '
             'eric@where.test,John <jdoe@test>'),


What about the missing space before John on this and subsequent lines? I realize it's not the issue you're solving, but since you're adding spaces after commas, do you need one here, too? Or maybe the point of this test is to not have spaces after commas? I haven't looked at it closely yet.

These tests is the only thing about which I am unsure.

The email team should look and decide. This again, is structured data, not English prose.

I'm not 100% positive, but I think it's okay in this case. I would wait for comment from @bitdancer though.

I think you should leave all the email tests alone. Having a mixture of spaces after commas and no spaces after commas is good thing in these tests, and although I don't remember for sure, most likely intentional.

Lib/test/test_http_cookiejar.py

Lib/unittest/mock.py

terryjreedy

The fixes for list errors and English misspellings (other than the assert failure message) look correct.

I think an email person (@bitdancer ) should look at the email test data formatting -- or omit the changes.

terryjreedy · 2018-11-01T18:24:52Z

Lib/email/_header_value_parser.py

@@ -2209,7 +2209,7 @@ def get_section(value):
        digits += value[0]
        value = value[1:]
    if digits[0] == '0' and digits != '0':
-        section.defects.append(errors.InvalidHeaderError("section number"
+        section.defects.append(errors.InvalidHeaderError("section number "
            "has an invalid leading 0"))


section.defects.append(errors.InvalidHeaderError("section number " "has an invalid leading 0"))

could evade the issue and be a bit more readable with

section.defects.append(errors.InvalidHeaderError( "section number has an invalid leading 0"))

Lib/idlelib/idle_test/htest.py

Lib/test/test_email/test__header_value_parser.py

terryjreedy · 2018-11-01T18:24:52Z

Lib/test/test_email/test__header_value_parser.py

@@ -2154,11 +2154,11 @@ def test_get_group_one_invalid(self):

    def test_get_group_missing_final_semicol(self):
        group = self._test_get_x(parser.get_group,
-            ('Monty Python:"Fred A. Bear" <dinsdale@example.com>,'
+            ('Monty Python:"Fred A. Bear" <dinsdale@example.com>, '
             'eric@where.test,John <jdoe@test>'),


The email team should look and decide. This again, is structured data, not English prose.

Lib/test/test_http_cookiejar.py

Lib/test/test_email/test__header_value_parser.py

bitdancer · 2018-11-01T20:12:08Z

Lib/test/test_email/test__header_value_parser.py

@@ -2154,11 +2154,11 @@ def test_get_group_one_invalid(self):

    def test_get_group_missing_final_semicol(self):
        group = self._test_get_x(parser.get_group,
-            ('Monty Python:"Fred A. Bear" <dinsdale@example.com>,'
+            ('Monty Python:"Fred A. Bear" <dinsdale@example.com>, '
             'eric@where.test,John <jdoe@test>'),


I think you should leave all the email tests alone. Having a mixture of spaces after commas and no spaces after commas is good thing in these tests, and although I don't remember for sure, most likely intentional.

bedevere-bot · 2018-11-01T20:12:15Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

serhiy-storchaka · 2018-11-01T21:00:57Z

I have made the requested changes; please review again.

bedevere-bot · 2018-11-01T21:01:00Z

Thanks for making the requested changes!

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

asottile

this is amazing, how did you find all of these? is there a tool or was it through some grep?

serhiy-storchaka · 2018-11-05T12:35:10Z

There are tools for multiline searching, but it was easier to me to write a simple Python script.

terryjreedy

The changes requested by myself and David Murray have be made.

The changes to test_email have be made as requested.

terryjreedy · 2018-11-05T14:18:35Z

@bitdancer Since you have not re-reviewed since 44c6760 removed the changes to test_email, 4 days ago, I guessed that you must be busy. So your request was easy to confirm, I took the initiative of doing so in your behalf and dismissing your negative review, and giving Serhiy an OK to merge. If this annoys you, let me know that I should not repeat.

miss-islington · 2018-11-05T14:20:28Z

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

bedevere-bot · 2018-11-05T14:20:48Z

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

serhiy-storchaka · 2018-11-05T14:20:49Z

Thank you all for your reviews!

miss-islington · 2018-11-05T14:20:49Z

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 34fd4c20198dea6ab2fe8dc6d32d744d9bde868d 3.6

… lines. (pythonGH-10284) Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings. (cherry picked from commit 34fd4c2) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

… lines. (GH-10284) Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings. (cherry picked from commit 34fd4c2) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…ferent lines. (pythonGH-10284) Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings.. (cherry picked from commit 34fd4c2) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-bot · 2018-11-05T14:56:07Z

GH-10335 is a backport of this pull request to the 3.6 branch.

…ferent lines. (GH-10284) (GH-10335) Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings.. (cherry picked from commit 34fd4c2)

…ferent lines. (pythonGH-10284) (pythonGH-10335) Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings.. (cherry picked from commit 34fd4c2). (cherry picked from commit 7054e5c)

…ferent lines. (GH-10284) (GH-10335) (GH-10336) Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings. (cherry picked from commit 34fd4c2) (cherry picked from commit 7054e5c)

bpo-35133: Fix mistakes when concatenate string literals on different…

f6c9934

… lines. Two kind of mistakes: 1. Missed space. After concatenating there is no space between words. 2. Missed comma. Causes unintentional concatenating in a list of strings.

serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.6 skip news needs backport to 3.7 labels Nov 1, 2018

serhiy-storchaka requested review from rhettinger, skrah, terryjreedy and a team as code owners Nov 1, 2018

serhiy-storchaka requested a review from a team Nov 1, 2018

the-knights-who-say-ni added the CLA signed label Nov 1, 2018

bedevere-bot added the awaiting merge label Nov 1, 2018

ericvsmith reviewed Nov 1, 2018

View changes

Remove a dot.

64020f7

tirkarthi reviewed Nov 1, 2018

View changes

Lib/unittest/mock.py Show resolved Hide resolved

terryjreedy reviewed Nov 1, 2018

View changes

bitdancer previously requested changes Nov 1, 2018

View changes

bedevere-bot removed the awaiting merge label Nov 1, 2018

bedevere-bot added the awaiting changes label Nov 1, 2018

serhiy-storchaka added 2 commits Nov 1, 2018

Restore email tests.

44c6760

Rewrite _header_value_parser.py as Terry suggested.

2417e92

bedevere-bot added awaiting change review and removed awaiting changes labels Nov 1, 2018

asottile approved these changes Nov 3, 2018

View changes

terryjreedy approved these changes Nov 5, 2018

View changes

bedevere-bot removed the awaiting change review label Nov 5, 2018

bedevere-bot added the awaiting merge label Nov 5, 2018

serhiy-storchaka merged commit 34fd4c2 into python:master Nov 5, 2018

bedevere-bot removed the awaiting merge label Nov 5, 2018

serhiy-storchaka deleted the string-literals-concat-multiline branch Nov 5, 2018

bedevere-bot removed the needs backport to 3.7 label Nov 5, 2018

miss-islington assigned serhiy-storchaka Nov 5, 2018

bedevere-bot removed the needs backport to 3.6 label Nov 5, 2018

serhiy-storchaka removed their assignment Nov 5, 2018

bpo-35133: Fix mistakes when concatenate string literals on different lines. #10284

bpo-35133: Fix mistakes when concatenate string literals on different lines. #10284

serhiy-storchaka commented Nov 1, 2018 •

edited by bedevere-bot

ericvsmith left a comment

ericvsmith Nov 1, 2018

serhiy-storchaka Nov 1, 2018

terryjreedy Nov 1, 2018

warsaw Nov 1, 2018

bitdancer Nov 1, 2018

terryjreedy left a comment

terryjreedy Nov 1, 2018

terryjreedy Nov 1, 2018

bitdancer Nov 1, 2018

bedevere-bot commented Nov 1, 2018

serhiy-storchaka commented Nov 1, 2018

bedevere-bot commented Nov 1, 2018

asottile left a comment

serhiy-storchaka commented Nov 5, 2018

terryjreedy left a comment

terryjreedy commented Nov 5, 2018

miss-islington commented Nov 5, 2018

bedevere-bot commented Nov 5, 2018

serhiy-storchaka commented Nov 5, 2018

miss-islington commented Nov 5, 2018

bedevere-bot commented Nov 5, 2018

bpo-35133: Fix mistakes when concatenate string literals on different lines. #10284

bpo-35133: Fix mistakes when concatenate string literals on different lines. #10284

Conversation

serhiy-storchaka commented Nov 1, 2018 • edited by bedevere-bot

ericvsmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terryjreedy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-bot commented Nov 1, 2018

serhiy-storchaka commented Nov 1, 2018

bedevere-bot commented Nov 1, 2018

asottile left a comment

serhiy-storchaka commented Nov 5, 2018

terryjreedy left a comment

terryjreedy commented Nov 5, 2018

miss-islington commented Nov 5, 2018

bedevere-bot commented Nov 5, 2018

serhiy-storchaka commented Nov 5, 2018

miss-islington commented Nov 5, 2018

bedevere-bot commented Nov 5, 2018

serhiy-storchaka commented Nov 1, 2018 •

edited by bedevere-bot