Addressing the review comments by Antoine Pitrou for smtplib.py and test_smtplib.py. Review comments by Ezio Melotti for smtplib.rst

3 files changed, 39 insertions, 28 deletions

diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst
index 4a0f756..7dd038d 100644
--- a/Doc/library/smtplib.rst
+++ b/Doc/library/smtplib.rst
@@ -31,9 +31,9 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions).
    like the connection attempt (if not specified, the global default timeout
    setting will be used). The optional source_address parameter allows to bind to some
    specific source address in a machine with multiple network interfaces,
-   and/or to some specific source tcp port. It takes a 2-tuple (host, port),
+   and/or to some specific source TCP port. It takes a 2-tuple (host, port),
    for the socket to bind to as its source address before connecting. If
-   ommited (or if host or port are '' and/or 0 respectively) the OS default
+   omitted (or if host or port are ``''`` and/or 0 respectively) the OS default
    behavior will be used.
 
    For normal use, you should only require the initialization/connect,
@@ -53,8 +53,8 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions).
    .. versionchanged:: 3.3
       Support for the :keyword:`with` statement was added.
 
-   .. versionadded:: 3.3
-      source_address parameter.
+   .. versionchanged:: 3.3
+      source_address argument was added.
 
 .. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None, source_address=None)
 
@@ -73,14 +73,14 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions).
    specific source address in a machine with multiple network interfaces,
    and/or to some specific source tcp port. It takes a 2-tuple (host, port),
    for the socket to bind to as its source address before connecting. If
-   ommited (or if host or port are '' and/or 0 respectively) the OS default
+   omitted (or if host or port are ``''`` and/or 0 respectively) the OS default
    behavior will be used.
 
    .. versionchanged:: 3.3
       *context* was added.
 
-   .. versionadded:: 3.3
-      source_address parameter.
+   .. versionchanged:: 3.3
+      source_address argument was added.
 
 
 .. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None, source_address=None)
@@ -88,8 +88,8 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions).
    The LMTP protocol, which is very similar to ESMTP, is heavily based on the
    standard SMTP client. It's common to use Unix sockets for LMTP, so our
    :meth:`connect` method must support that as well as a regular host:port
-   server. The optional parameters local_hostname and source_address has the
-   same meaning as that of SMTP client.To specify a Unix socket, you must use
+   server. The optional arguments local_hostname and source_address have the
+   same meaning as that of SMTP client. To specify a Unix socket, you must use
    an absolute path for *host*, starting with a '/'.
 
    Authentication is supported, using the regular SMTP mechanism. When using a Unix
diff --git a/Lib/smtplib.py b/Lib/smtplib.py
index 2039592..a4c0d89 100644
--- a/Lib/smtplib.py
+++ b/Lib/smtplib.py
@@ -282,7 +282,8 @@ class SMTP:
         # This makes it simpler for SMTP_SSL to use the SMTP connect code
         # and just alter the socket connection bit.
         if self.debuglevel > 0:
-            print('connect: to', (host, port), self.source_address, file=stderr)
+            print('connect: to', (host, port), self.source_address,
+                                 file=stderr)
         return socket.create_connection((host, port), timeout,
                                         self.source_address)
 
@@ -297,7 +298,10 @@ class SMTP:
         specified during instantiation.
 
         """
-        if source_address: self.source_address = source_address
+
+        if source_address:
+            self.source_address = source_address
+
         if not port and (host.find(':') == host.rfind(':')):
             i = host.rfind(':')
             if i >= 0:
@@ -381,7 +385,8 @@ class SMTP:
 
         errmsg = b"\n".join(resp)
         if self.debuglevel > 0:
-            print('reply: retcode (%s); Msg: %s' % (errcode, errmsg), file=stderr)
+            print('reply: retcode (%s); Msg: %s' % (errcode, errmsg),
+                                                    file=stderr)
         return errcode, errmsg
 
     def docmd(self, cmd, args=""):
@@ -788,7 +793,8 @@ class SMTP:
         # TODO implement heuristics to guess the correct Resent-* block with an
         # option allowing the user to enable the heuristics.  (It should be
         # possible to guess correctly almost all of the time.)
-        resent =msg.get_all('Resent-Date')
+
+        resent = msg.get_all('Resent-Date')
         if resent is None:
             header_prefix = ''
         elif len(resent) == 1:
@@ -797,13 +803,13 @@ class SMTP:
             raise ValueError("message has more than one 'Resent-' header block")
         if from_addr is None:
             # Prefer the sender field per RFC 2822:3.6.2.
-            from_addr = (msg[header_prefix+'Sender']
-                           if (header_prefix+'Sender') in msg
-                           else msg[header_prefix+'From'])
+            from_addr = (msg[header_prefix + 'Sender']
+                           if (header_prefix + 'Sender') in msg
+                           else msg[header_prefix + 'From'])
         if to_addrs is None:
-            addr_fields = [f for f in (msg[header_prefix+'To'],
-                                       msg[header_prefix+'Bcc'],
-                                       msg[header_prefix+'Cc']) if f is not None]
+            addr_fields = [f for f in (msg[header_prefix + 'To'],
+                                       msg[header_prefix + 'Bcc'],
+                                       msg[header_prefix + 'Cc']) if f is not None]
             to_addrs = [a[1] for a in email.utils.getaddresses(addr_fields)]
         # Make a local copy so we can delete the bcc headers.
         msg_copy = copy.copy(msg)
@@ -899,13 +905,13 @@ class LMTP(SMTP):
     def __init__(self, host='', port=LMTP_PORT, local_hostname=None,
             source_address=None):
         """Initialize a new instance."""
-        SMTP.__init__(self, host, port, local_hostname = local_hostname,
-                source_address = source_address)
+        SMTP.__init__(self, host, port, local_hostname=local_hostname,
+                      source_address=source_address)
 
     def connect(self, host='localhost', port=0, source_address=None):
         """Connect to the LMTP daemon, on either a Unix or a TCP socket."""
         if host[0] != '/':
-            return SMTP.connect(self, host, port, source_address = source_address)
+            return SMTP.connect(self, host, port, source_address=source_address)
 
         # Handle Unix-domain sockets.
         try:
diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py
index 70654c5..969ff15 100644
--- a/Lib/test/test_smtplib.py
+++ b/Lib/test/test_smtplib.py
@@ -216,12 +216,17 @@ class DebuggingServerTests(unittest.TestCase):
 
     def testSourceAddress(self):
         # connect
-        smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3,
-                            source_address=('127.0.0.1', 19876))
-        self.assertEqual(smtp.source_address, ('127.0.0.1', 19876))
-        self.assertEqual(smtp.local_hostname, 'localhost')
-        print(dir(smtp))
-        smtp.quit()
+        port = support.find_unused_port()
+        try:
+            smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost',
+                    timeout=3, source_address=('127.0.0.1', port))
+            self.assertEqual(smtp.source_address, ('127.0.0.1', port))
+            self.assertEqual(smtp.local_hostname, 'localhost')
+            smtp.quit()
+        except IOError as e:
+            if e.errno == errno.EADDRINUSE:
+                self.skipTest("couldn't bind to port %d" % port)
+            raise
 
     def testNOOP(self):
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)