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

Fix crash for non breaking spaces in docstring #2099

Closed
wants to merge 1 commit into from

Conversation

Pierre-Sassoulas
Copy link
Contributor

@Pierre-Sassoulas Pierre-Sassoulas commented Apr 11, 2021

This is a follow up of #2092. (Work in progress)

Closes #1762

@ichard26
Copy link
Collaborator

ichard26 commented Apr 11, 2021

I'mma attach a skip news label under the impression that this PR finishes or fixes what was set out in PR GH-2092, which hasn't been released yet so a changelog entry isn't necessary.

edit: I also recommend marking this PR as a draft

@ichard26 ichard26 added the skip news Pull requests that don't need a changelog entry. label Apr 11, 2021
@JelleZijlstra
Copy link
Collaborator

I feel like this should actually get a different news entry because it's about nbsps in docstrings, not comments.

@JelleZijlstra JelleZijlstra marked this pull request as draft April 11, 2021 22:59
@ichard26 ichard26 removed the skip news Pull requests that don't need a changelog entry. label Apr 11, 2021
@ichard26
Copy link
Collaborator

Ah, I was under the impression that PR GH-2092 had an unfixed bug which this PR fixes, now looking into it you are right @JelleZijlstra, thanks for letting me know!

@ichard26
Copy link
Collaborator

I was going through the issue tracker and found this issue: #1762, it seems to be what this PR aims to fix. If so, please link it so we don't have to remember to go close that issue when this is merged. Thank you in advance!

@Pierre-Sassoulas
Copy link
Contributor Author

Funny, I opened this issue in October. I was thinking it might be the same problem. Thank you for digging it !

@ambv
Copy link
Collaborator

ambv commented Apr 27, 2021

I added some more tests on this and found it to be fixed: eaa337f

Let's reopen if needed but it looks solved. Thanks for your contribution.

@ambv ambv closed this Apr 27, 2021
@Pierre-Sassoulas
Copy link
Contributor Author

I just tested on the test I created, everything is working (no more crash for sure), thank you for fixing this so fast :)

Maybe a little something unrelated to the bug, with this input:

def function(a:int=42):
    """ This docstring start with a NBSP
        There is one NBSP column 5 here
       This is 7 NBSP
    """

    #    There's a NBSP + 3 spaces before
    #    And 4 spaces on the next line
    pass

Here's the diff compared to what I was expecting:

    def function(a: int = 42):
        """This docstring start with a NBSP
-     There is one NBSP column 5 here
+      There is one NBSP column 5 here
      This is 7 NBSP
      """
  
      #    There's a NBSP + 3 spaces before
      #    And 4 spaces on the next line
      pass

Probably because there is 7 spaces and not 8 on the last line of the docstring ? I'm going to close #1762 either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

INTERNAL ERROR when there is an non breaking space in a docstring
4 participants