bpo-30103: Allow Uuencode in Python using backtick as zero instead of space #1326

zhangyangyu · 2017-04-27T16:08:14Z

No description provided.

mention-bot · 2017-04-27T16:08:17Z

@zhangyangyu, thanks for your PR! By analyzing the history of the files in this pull request, we identified @doerwalter, @serhiy-storchaka and @warsaw to be potential reviewers.

serhiy-storchaka · 2017-04-28T07:26:55Z

Doc/whatsnew/3.7.rst

+
+The :func:`~binascii.b2a_uu` function now accepts an optional *backtick*
+keyword argument.  When it's true, zeros are represented by backticks
+instead of spaces. (Contributed by Xiang Zhang in :issue:`30103`.)


Double space after a period.

serhiy-storchaka · 2017-04-28T07:33:32Z

Lib/test/test_binascii.py

+                         b'$`$-A=```\n')
+        self.assertEqual(binascii.a2b_uu(b'$`$-A=```\n'),
+                         binascii.a2b_uu(b'$ $-A=   \n'))
+        self.assertRaises(TypeError, binascii.b2a_uu, b'', b'`\n', True)


What does this test mean?

backtick is expected to be keyword-only. But sorry I make some mistake here.

serhiy-storchaka · 2017-04-28T07:49:46Z

Lib/test/test_uu.py

+def encodedtextwrapped(mode, filename, backtick=False):
+    if backtick:
+        res = (bytes("begin %03o %s\n" % (mode, filename), "ascii") +
+               encodedtext.replace(b' ', b'`') + b"\n`\nend\n")


Seems the only space in encodedtext is the padding space. It would be worth to change examples so that they include inner spaces.

Good catch. I found it when write the test too but didn't change it.

serhiy-storchaka · 2017-04-28T07:55:01Z

Modules/binascii.c

@@ -367,7 +368,10 @@ binascii_b2a_uu_impl(PyObject *module, Py_buffer *data)
        return NULL;

    /* Store the length */
-    *ascii_data++ = ' ' + (bin_len & 077);
+    if (backtick)


May be

if (backtick && !bin_len) *ascii_data++ = '`'; else *ascii_data++ = ' ' + (bin_len & 077);

?

It's good. The code seems more clear telling the logic that only zero needs to be specially treated.

vadmium · 2017-04-29T02:14:59Z

Lib/test/test_uu.py


 encodedtext = b"""\
-M5&AE('-M;V]T:\"US8V%L960@<'ET:&]N(&-R97!T(&]V97(@=&AE('-L965P
-(:6YG(&1O9PH """
+M5&AE(\'-Y;6)O;\',@;VX@=&]P(&]F(\'EO=7(@:V5Y8F]A<F0@87)E("% (R0E


I don’t think you need to escape the quotes (\')

vadmium · 2017-04-29T02:19:34Z

Modules/binascii.c

@@ -334,14 +334,15 @@ binascii_a2b_uu_impl(PyObject *module, Py_buffer *data)
 binascii.b2a_uu

    data: Py_buffer
-    /


Was this intended? If so, it needs documenting and testing, but it would be good to avoid if possible. Can’t you put a slash and asterisk after each other to have positional-only and then keyword-only parameters?

AC currently seems doesn't support combining positional-only parameters with keyword-only parameters. So it's not intended but a side effect.

It does, I just checked this.

While I support keeping data positional-only, but in bpo-25357 b2a_base64 was changed and as a side effect data is able to accept a keyword argument. The consistency that functions in module binascii all support a positional-only data was broken. What's your mind @serhiy-storchaka and @vadmium ? We keep data positional-only or just make it like b2a_base64 or make b2a_base64 data back?

It looks to me that change in bpo-25357 was not intentional. Perhaps we can revert it. Open a new issue for this.

vadmium · 2017-04-29T03:11:47Z

Lib/test/test_uu.py

-                              UUStdIOTest,
-                              UUFileTest,
-                              )
+    support.run_unittest(UUTest, UUStdIOTest, UUFileTest)


This seems like an insignificant change. Wouldn’t it be simple to use “unittest.main” instead?

serhiy-storchaka · 2017-04-29T08:33:18Z

Doc/library/binascii.rst


   Convert binary data to a line of ASCII characters, the return value is the
   converted line, including a newline char. The length of *data* should be at most
-   45.
+   45. If *backtick* is true, zeros are represented by backticks instead of spaces.


Show the backtick character explicitly, as

``"`"``

And in all other cases where it is mentioned.

serhiy-storchaka · 2017-04-29T08:34:10Z

Doc/library/binascii.rst

@@ -40,11 +40,14 @@ The :mod:`binascii` module defines the following functions:
   data may be followed by whitespace.


-.. function:: b2a_uu(data)
+.. function:: b2a_uu(data, \*, backtick=False)


AFAIK the backslash is not needed here.

serhiy-storchaka · 2017-04-29T08:43:28Z

Modules/binascii.c

+    if (backtick && !bin_len)
+        *ascii_data++ = '`';
+    else
+        *ascii_data++ = ' ' + (bin_len & 077);


Actually & 077 is not needed. bin_len is not larger than 45.

zhangyangyu · 2017-05-03T03:21:19Z

Thanks @serhiy-storchaka and @vadmium !

the-knights-who-say-ni added the CLA signed label Apr 27, 2017

allow Uuencode using backticks instead of spaces to represent zeros

a45a02b

zhangyangyu force-pushed the bpo-30103 branch from d3c386b to a45a02b Compare April 28, 2017 03:43

serhiy-storchaka reviewed Apr 28, 2017

View reviewed changes

address Serhiy's comments

7f39aee

vadmium reviewed Apr 29, 2017

View reviewed changes

zhangyangyu added 2 commits April 29, 2017 15:00

address Martin's comments

9cf21f6

restore data to positional-only

5cf9c40

serhiy-storchaka reviewed Apr 29, 2017

View reviewed changes

zhangyangyu added 6 commits April 29, 2017 17:16

address Serhiy's comments

8be2e95

remove unnecessary backslash

7c4f0e6

remove positional-only test

4b387f2

Merge remote-tracking branch 'upstream/master' into bpo-30103

fd64af5

Merge remote-tracking branch 'upstream/master' into bpo-30103

e2d0112

Merge remote-tracking branch 'upstream/master' into bpo-30103

2e43ff1

zhangyangyu merged commit 13f1f42 into python:master May 3, 2017

zhangyangyu deleted the bpo-30103 branch May 3, 2017 03:16

Uh oh!

bpo-30103: Allow Uuencode in Python using backtick as zero instead of space #1326

bpo-30103: Allow Uuencode in Python using backtick as zero instead of space #1326

Uh oh!

Conversation

zhangyangyu commented Apr 27, 2017

Uh oh!

mention-bot commented Apr 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vadmium Apr 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangyangyu Apr 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangyangyu commented May 3, 2017

Uh oh!

Uh oh!

vadmium Apr 29, 2017 •

edited

Loading

zhangyangyu Apr 29, 2017 •

edited

Loading