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

Correct the declarations of I/O buffers as bytes-based #1400

Merged
merged 2 commits into from Nov 1, 2023

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Nov 1, 2023

  • Correctly declare the parameter annotations as BinaryIO, because in fact it works only with byte-based buffers.

    Attempt at running code both before and after this change:

    >>> from markdown import Markdown
    
    >>> from io import StringIO, BytesIO
    
    >>> Markdown().convertFile(BytesIO(b'input'), output=BytesIO())
    <markdown.core.Markdown object at 0x7fe3bba7c710>
    
    >>> Markdown().convertFile(StringIO('input'), output=BytesIO())
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "markdown/core.py", line 423, in convertFile
        text = input_file.read()
              ^^^^^^^^^^^^^^^^^
      File "<frozen codecs>", line 500, in read
    TypeError: can't concat str to bytes
    
    >>> Markdown().convertFile(BytesIO(b'input'), output=StringIO())
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "markdown/core.py", line 446, in convertFile
        output_file.write(html)
      File "<frozen codecs>", line 378, in write
    TypeError: string argument expected, got 'bytes'
  • sys.stdin.read() is always a string, so remove the Python2-specific check.

    • Correct the patch in the test which incorrectly sets sys.stdin to a bytes buffer which it never is.
  • sys.stdout.buffer always exists, so remove the Python2-specific fallback.

    • Correct the patch in the test which incorrectly sets sys.stdout to a bytes buffer which it never is. And fix a matching mistake because this necessitated stdout.read().decode().

* Correctly declare the parameter annotations as `BinaryIO`, because in fact it works only with byte-based buffers.

* `sys.stdin.read()` is always a string, so remove the Python2-specific check.
    * Correct the patch in the test which incorrectly sets `sys.stdin` to a bytes buffer which it never is.

* `sys.stdout.buffer` always exists, so remove the Python2-specific fallback.
    * Correct the patch in the test which incorrectly sets `sys.stdout` to a bytes buffer which it never is. And fix a matching mistake because this necessitated `stdout.read().decode()`.
Copy link
Member

@waylan waylan left a comment

Choose a reason for hiding this comment

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

Thanks for this (along with the explanation. I've been meaning to update this to remove the old legacy code and never remembered when I had the time/motivation.

markdown/core.py Outdated Show resolved Hide resolved
@waylan waylan merged commit 8e517de into Python-Markdown:master Nov 1, 2023
16 checks passed
@oprypin oprypin deleted the bytesio branch November 1, 2023 15:19
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.

None yet

2 participants