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

[Martin von Zweigbergk]

On Thu Nov 06 2014 at 7:47:17 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 1415275068 0
> #      Thu Nov 06 11:57:48 2014 +0000
> # Node ID 1c2a64da69f91d37af44d0126ea2c3d73b3a8790
> # Parent  2d48ca81fd12986424d08a7abd6fdaeaa1d832d7
> patchbomb: extract 'makeintro' 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
> @@ -255,10 +255,44 @@ def _getbundlemsgs(repo, sender, bundle,
>      email.Encoders.encode_base64(datapart)
>      msg.attach(datapart)
>      msg['Subject'] = mail.headencode(ui, subj, _charsets,
> opts.get('test'))
>      return [(msg, subj, None)]
>
> +def _makeintro(repo, sender, patches, **opts):
> +    """make an introduction email, asking the using for a content if
> needed
> +
> +    email is returned as (subject, body, cumulative-diffstat)"""
> +    ui = repo.ui

+    _charsets = mail._charsets(ui)
>

Move closer to its first use (and after some early returns) unless you plan
on doing that later?

+    tlen = len(str(len(patches)))
> +
> +    flag = opts.get('flag') or ''
> +    if flag:
> +        flag = ' ' + ' '.join(flag)
> +    prefix = '[PATCH %0*d of %d%s]' % (tlen, 0, len(patches), flag)
> +
> +    subj = (opts.get('subject') or
> +            prompt(ui, '(optional) Subject: ', rest=prefix, default=''))
> +    if not subj:
> +        return None         # skip intro if the user doesn't bother
> +
> +    subj = prefix + ' ' + subj
> +
> +    body = ''
> +    if opts.get('diffstat'):
> +        # generate a cumulative diffstat of the whole patch series
> +        diffstat = patch.diffstat(sum(patches, []))
> +        body = '\n' + diffstat
> +    else:
> +        diffstat = None
> +
> +    body = _getdescription(repo, body, sender, **opts)
> +    msg = mail.mimeencode(ui, body, _charsets, opts.get('test'))
> +    msg['Subject'] = mail.headencode(ui, subj, _charsets,
> +                                     opts.get('test'))
> +    return (msg, subj, diffstat)
> +
>  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')),
> @@ -423,11 +457,11 @@ def patchbomb(ui, repo, *revs, **opts):
>          ui.write(_('this patch series consists of %d patches.\n\n')
>                   % len(patches))
>
>          # build the intro message, or skip it if the user declines
>          if introwanted(opts, len(patches)):
> -            msg = makeintro(patches)
> +            msg = _makeintro(repo, sender, patches, **opts)
>              if msg:
>                  msgs.append(msg)
>
>          # are we going to send more than one message?
>          numbered = len(msgs) + len(patches) > 1
> @@ -441,38 +475,10 @@ def patchbomb(ui, repo, *revs, **opts):
>                              len(patches), numbered, name)
>              msgs.append(msg)
>
>          return msgs
>
> -    def makeintro(patches):
> -        tlen = len(str(len(patches)))
> -
> -        flag = opts.get('flag') or ''
> -        if flag:
> -            flag = ' ' + ' '.join(flag)
> -        prefix = '[PATCH %0*d of %d%s]' % (tlen, 0, len(patches), flag)
> -
> -        subj = (opts.get('subject') or
> -                prompt(ui, '(optional) Subject: ', rest=prefix,
> default=''))
> -        if not subj:
> -            return None         # skip intro if the user doesn't bother
> -
> -        subj = prefix + ' ' + subj
> -
> -        body = ''
> -        if opts.get('diffstat'):
> -            # generate a cumulative diffstat of the whole patch series
> -            diffstat = patch.diffstat(sum(patches, []))
> -            body = '\n' + diffstat
> -        else:
> -            diffstat = None
> -
> -        body = _getdescription(repo, body, sender, **opts)
> -        msg = mail.mimeencode(ui, body, _charsets, opts.get('test'))
> -        msg['Subject'] = mail.headencode(ui, subj, _charsets,
> -                                         opts.get('test'))
> -        return (msg, subj, diffstat)
>
>      sender = (opts.get('from') or ui.config('email', 'from') or
>                ui.config('patchbomb', 'from') or
>                prompt(ui, 'From', ui.username()))
>
> _______________________________________________
> 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/3a33c7c5/attachment.html>