This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author belopolsky
Recipients belopolsky, eric.araujo, georg.brandl, pitrou, vstinner
Date 2010-11-08.15:28:12
SpamBayes Score 9.65894e-15
Marked as misclassified No
Message-id <AANLkTin-3NEdEAkN8MbfQ+o+F4mHinuZTQHLJ233NHS6@mail.gmail.com>
In-reply-to <201011070536.00599.victor.stinner@haypocalc.com>
Content
On Sun, Nov 7, 2010 at 4:24 AM, STINNER Victor <report@bugs.python.org> wrote:
..
> Ok, the new patch (tokenize_open-2.patch) uses tokenize.open() name and adds a
> test for BOM without coding cookie (test utf-8-sig encoding).

Here are my comments on the new patch:

1. A nit-pick: tokenize.py is already inconsistent in this respect,
but I believe it is conventional not to start a docsting with an empty
line:

+def open(filename):
+    """Open a Python script in read mode with the right encoding.

instead of

+def open(filename):
+    """
+    Open a Python script in read mode with the right encoding.

Also, I would prefer a more neutral description that will not imply
that the method is only useful for Python source code.  For example:

"""Detect encoding and open a file in read mode with the right encoding.

(optionally a more detailed explanation or a reference to detect_encoding.)
"""

2. Another nit-pick: detect_encoding() returns a list of 0-2 lines in
the second item, not a single line, so it is better to write

+    encoding, lines = detect_encoding(buffer.readline)

or even

+    encoding, _ = detect_encoding(buffer.readline)

instead of

+    encoding, line = detect_encoding(buffer.readline)

3. In test_open(self), I think you should use self.assertEqual()
instead of a plain assert.

4. Last nit-pick (feel free to ignore as prejudice to %-formatting :-):

instead of

+                print("# coding: %s" % encoding, file=fp)

you can write

+                print("# coding: " + encoding, file=fp)
History
Date User Action Args
2010-11-08 15:28:16belopolskysetrecipients: + belopolsky, georg.brandl, pitrou, vstinner, eric.araujo
2010-11-08 15:28:13belopolskylinkissue10335 messages
2010-11-08 15:28:12belopolskycreate