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

🪲 Inconsistent handling of trailing spaces in assignments in levels 4-11 #5684

Open
boryanagoncharenko opened this issue Aug 5, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@boryanagoncharenko
Copy link
Collaborator

boryanagoncharenko commented Aug 5, 2024

Describe the bug
This issue is a branch of #3287 focusing specifically on the inconsistent parsing of assignments with trailing comments. The following program (levels 9-11) should produce consistent result

i = 4   # met comment
print '[' i ']'
if 1 = 1
    j = 4    # met comment
    print '[' j ']'

But it outputs:

[4]
[4   ]

Add a screenshot (optional)
Screenshot 2024-08-05 at 16 02 06

Considerations
Up until level 12, spaces are allowed on the right-hand-side of assignments without quotes. While this makes it natural to count all trailing spaces as part of the value, the text editor trims all spaces before sending the request, which means that the users are used to the automatic trimming. A clear cut example is when there is a trailing comment: are the trailing spaces part of the value or the comment?

if 1 = 1
    var = that is    # met comment
                  ^^^
    is this part of the value or the comment

We could easily resolve this by changing the grammar definition of textwithspaces, so that it cannot end with a space. However, then the following test starts failing:

    def test_assign_list_with_spaces(self):
        # spaces are parsed in the text here, that is fine (could be avoided if we say text
        # can't *end* (or start) in a space but I find this ok for now
        code = "dieren is Hond , Kat , Kangoeroe"
        expected = "dieren = ['Hond ', 'Kat ', 'Kangoeroe']"

        self.multi_level_tester(max_level=11, code=code, expected=expected, unused_allowed=True)

Perhaps we could reconsider? Especially when the editor is trimming the lines and leaving trailing spaces could make equality checks confusing.

@Felienne
Copy link
Member

Perhaps we could reconsider? Especially when the editor is trimming the lines and leaving trailing spaces could make equality checks confusing.

I can totally live with reconsidering, and thanks for the superdetailed explanation!

Just to be sure, do you propose to change the code such that the now failing test would be rewritten to this:

     def test_assign_list_with_spaces(self):
        # spaces are parsed in the text here, that is fine (could be avoided if we say text
        # can't *end* (or start) in a space but I find this ok for now
        code = "dieren is Hond , Kat , Kangoeroe"
        expected = "dieren = ['Hond', 'Kat', 'Kangoeroe']" #<---- with no spaces here

        self.multi_level_tester(max_level=11, code=code, expected=expected, unused_allowed=True)

I see how that would be logically, we then remove all "trailing" spaces not just at the end of the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: ToBeDiscussed
Development

No branches or pull requests

2 participants