Re: [PATCH 5 of 7 🚿🍦] patchbomb: extract 'getbundlemsgs' closure in its own function

[Martin von Zweigbergk]

On Thu Nov 06 2014 at 7:47:08 AM Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1415137703 0
> #      Tue Nov 04 21:48:23 2014 +0000
> # Node ID 2d48ca81fd12986424d08a7abd6fdaeaa1d832d7
> # Parent  7a4add054e1090a3aa88464cf1ca54751b9d3e36
> patchbomb: extract 'getbundlemsgs' closure in its own function
>
> Keep marching toward the promised land of simplification!
>
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -230,10 +230,35 @@ def _getdescription(repo, defaultbody, s
>          msgfile = repo.opener('last-email.txt', 'wb')
>          msgfile.write(body)
>          msgfile.close()
>      return body
>
> +def _getbundlemsgs(repo, sender, bundle, **opts):
> +    """Get the full email sending a given bundle
> +
> +    This function return a list of "email" tuple (subject, content, None).
> +    The list is alway one message long in that case.
>

s/alway/always/

Should the reader be assumed to be know that the "None" is for the
diffstat? (I'm perfectly fine with a "yes"; it was just not clear to me.)


> +    """
> +    ui = repo.ui
> +    _charsets = mail._charsets(ui)
> +    subj = (opts.get('subject')
> +            or prompt(ui, 'Subject:', 'A bundle for your repository'))
> +
> +    body = _getdescription(repo, '', sender, **opts)
> +    msg = email.MIMEMultipart.MIMEMultipart()
> +    if body:
> +        msg.attach(mail.mimeencode(ui, body, _charsets, opts.get('test')))
> +    datapart = email.MIMEBase.MIMEBase('application',
> 'x-mercurial-bundle')
> +    datapart.set_payload(bundle)
> +    bundlename = '%s.hg' % opts.get('bundlename', 'bundle')
> +    datapart.add_header('Content-Disposition', 'attachment',
> +                        filename=bundlename)
> +    email.Encoders.encode_base64(datapart)
> +    msg.attach(datapart)
> +    msg['Subject'] = mail.headencode(ui, subj, _charsets,
> opts.get('test'))
> +    return [(msg, subj, None)]
> +
>  emailopts = [
>      ('', 'body', None, _('send patches as inline message text
> (default)')),
>      ('a', 'attach', None, _('send patches as attachments')),
>      ('i', 'inline', None, _('send patches as inline attachments')),
>      ('', 'bcc', [], _('email addresses of blind carbon copy recipients')),
> @@ -445,36 +470,21 @@ def patchbomb(ui, repo, *revs, **opts):
>          msg = mail.mimeencode(ui, body, _charsets, opts.get('test'))
>          msg['Subject'] = mail.headencode(ui, subj, _charsets,
>                                           opts.get('test'))
>          return (msg, subj, diffstat)
>
> -    def getbundlemsgs(bundle):
> -        subj = (opts.get('subject')
> -                or prompt(ui, 'Subject:', 'A bundle for your repository'))
> -
> -        body = _getdescription(repo, '', sender, **opts)
> -        msg = email.MIMEMultipart.MIMEMultipart()
> -        if body:
> -            msg.attach(mail.mimeencode(ui, body, _charsets,
> opts.get('test')))
> -        datapart = email.MIMEBase.MIMEBase('application',
> 'x-mercurial-bundle')
> -        datapart.set_payload(bundle)
> -        bundlename = '%s.hg' % opts.get('bundlename', 'bundle')
> -        datapart.add_header('Content-Disposition', 'attachment',
> -                            filename=bundlename)
> -        email.Encoders.encode_base64(datapart)
> -        msg.attach(datapart)
> -        msg['Subject'] = mail.headencode(ui, subj, _charsets,
> opts.get('test'))
> -        return [(msg, subj, None)]
> -
>      sender = (opts.get('from') or ui.config('email', 'from') or
>                ui.config('patchbomb', 'from') or
>                prompt(ui, 'From', ui.username()))
>
>      if patches:
>          msgs = getpatchmsgs(patches, opts.get('patchnames'))
>      elif bundle:
> -        msgs = getbundlemsgs(_getbundle(repo, dest, **opts))
> +        bundledata = _getbundle(repo, dest, **opts)
> +        bundleopts = opts.copy()
> +        bundleopts.pop('bundle', None)  # already processed
>

Sorry, I didn't get this part. What happens without the pop()? Why is it
needed because the function is extracted?


> +        msgs = _getbundlemsgs(repo, sender, bundledata, **bundleopts)
>      else:
>          msgs = getpatchmsgs(list(_getpatches(repo, revs, **opts)))
>
>      showaddrs = []
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20141106/4bd57053/attachment.html>