[Python-Dev] cpython: #11731: simplify/enhance parser/generator API by introducing policy objects.

[R. David Murray]

On Mon, 18 Apr 2011 20:26:36 +0200, Georg Brandl <g.brandl at gmx.net> wrote:
> On 18.04.2011 20:00, r.david.murray wrote:
> 
> > diff --git a/Doc/library/email.parser.rst b/Doc/library/email.parser.rst
> > --- a/Doc/library/email.parser.rst
> > +++ b/Doc/library/email.parser.rst
> > @@ -112,8 +118,13 @@
> >     :class:`~email.message.Message` (see :mod:`email.message`).  The factory will
> >     be called without arguments.
> >  
> > -   .. versionchanged:: 3.2
> > -      Removed the *strict* argument that was deprecated in 2.4.
> > +   The *policy* keyword specifies a :mod:`~email.policy` object that controls a
> > +   number of aspects of the parser's operation.  The default policy maintains
> > +   backward compatibility.
> > +
> > +   .. versionchanged:: 3.3
> > +      Removed the *strict* argument that was deprecated in 2.4.  Added the
> > +      *policy* keyword.
> 
> Hmm, so *strict* wasn't actually removed in 3.2?

Correct.  I had previously checked in a versionchanged with the wrong
release number.  Should have corrected that separately.

[...]

> > +Policy objects are the mechanism used to provide the email package with the
> > +flexibility to handle all these disparate use cases,
> 
> Looks like something is missing from this sentence :)

Éric thought so too, but it reads fine to me.  Maybe it is colloquial
grammar and I'm just blind to it.  I can't now remember what his suggested
modification was, either.  I've rewritten it as:

    Policy objects give the email package the flexibility to handle all
    these disparate use cases.

[...]
 
> > +   >>> from email import msg_from_binary_file
> > +   >>> from email.generator import BytesGenerator
> > +   >>> import email.policy
> > +   >>> from subprocess import Popen, PIPE
> > +   >>> with open('mymsg.txt', 'b') as f:
> > +   >>>     msg = msg_from_binary_file(f, policy=email.policy.mbox)
> > +   >>> p = Popen(['sendmail', msg['To'][0].address], stdin=PIPE)
> > +   >>> g = BytesGenerator(p.stdin, email.policy.policy=SMTP)
> 
> That keyword arg doesn't look right.

Yep, I got that backward when I edited it.
 
> > +   >>> g.flatten(msg)
> > +   >>> p.stdin.close()
> > +   >>> rc = p.wait()
> 
> Also, if you put interactive prompts, please use them correctly ("..." prompt
> and one blank line for the with block).

Éric had me fix one example of that already.  What do you mean by "one
blank line" though?  Doctest doesn't require blank lines after the ...
lines, does it?

[...]

> > +   .. method:: handle_defect(obj, defect)
> > +
> > +      *obj* is the object on which to register the defect.
> 
> What kind of object is *obj*?

Whatever object is being used to represent the data being parsed when
the defect is found.  Right now that's always a Message, but that won't
continue to be true.  The rest of the documentation mentions or will
mention which objects have defect lists, and it felt like duplicating
that information here was a bad case of repetition (ie: one that doesn't
add much value compared to the danger of it getting out of date.)

Except for this last item, I've fixed everything and will shortly
check it in.

> Sorry for the long review.

It was a long patch.  No need to apologize.

--
R. David Murray           http://www.bitdance.com