[PATCH] patchbomb: add --no-intro option to disable 'PATCH 0/N' email

Matt Mackall mpm at selenic.com
Wed Sep 21 14:13:09 CDT 2011


On Mon, 2011-09-19 at 21:39 -0400, Greg Ward wrote:
> # HG changeset patch
> # User Greg Ward <greg at gerg.ca>
> # Date 1316480001 14400
> # Node ID 86ba151d49e7d8c80bf95902aae817f3a22080d1
> # Parent  353a1ba928f682a0a3e29bf54b90c7cdc2f98fef
> patchbomb: add --no-intro option to disable 'PATCH 0/N' email.
> 
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -72,7 +72,17 @@
>  
>  def introneeded(opts, number):
>      '''is an introductory message required?'''
> -    return number > 1 or opts.get('intro') or opts.get('desc')
> +    intro = opts.get('intro')
> +    no_intro = opts.get('no_intro')
> +    if intro is None and no_intro is None:
> +        # Caller specified neither --intro nor --no-intro.
> +        return number > 1 or opts.get('desc')
> +    elif intro is None:
> +        # Yuck: this means that in "hg email --intro --no-intro",
> +        # --intro wins regardless of order. Blame fancyopts.py. ;-(
> +        return not no_intro
> +    else:
> +        return intro

This seems overly-complicated and not quite complete. Let's look at some
tables..

Ideally, the logic should have been simply:

desc  intro needed
 n         n    <- we got this wrong
 y         y

Instead, it started as "always send an intro", which I complained about.
So the intro flag was introduced:

intro  desc  multi   intro needed
  n      n     n         n
  n      n     y         y
  n      y     *         y
  y      n     *         y
  y      y     *         y 
  
This logic is basically the trivial:

  return (intro or desc or multi)

As you can see, adding the intro flag was completely pointless. It's
simply a desc flag that doesn't take a description and just complicates
things.

And now, with the new flag, it should probably be something like:

intro no-intro desc multi   intro needed
  n      n       n    n        n
  n      n       n    y        y 
  n      n       y    *        y
  n      y       n    y        n
  n      y       y    *      conflict
  y      n       *    *        y
  y      y       *    *      conflict
 
Horrors: we've now taken something that should have been completely
trivial and made it possible for users to get errors. FYI, this new
logic can be reduced to something like:

  if nointro and intro:
    error
  if nointro and desc:
    error
  return intro or desc or (multi and not nointro)   

At the very least, we should deprecate the intro flag so that people
don't need to worry about its pointlessness.

But we could also just change the current behavior:

intro  desc  multi   intro needed
  n      n     n         n
  n      n     y         n   <- change this default
  n      y     *         y
  y      *     *         y
               ^
  ^            ` this column no longer matters
  ` deprecate this flag

..and get back to sanity. So the question is:

"Is it ok to make patchbomb not prompt for a description by default for
multiple patches?"

I suspect that would bother some folks. Sigh. Alternately, we could
change this annoying behavior:

  $ hg email -r -2:tip
  This patch series consists of 2 patches.

  Subject: [PATCH 0 of 2] <enter>
  Please enter a valid value.
  Subject: [PATCH 0 of 2] 

to:

  Subject: [PATCH 0 of 2] <enter>
  skipping intro

..which would probably not bother anyone and obviate the need for
no-intro. Also, we should fix this:

  $ hg email -r -2:tip < /dev/null
  This patch series consists of 2 patches.

  abort: Subject: [PATCH 0 of 2] Please enter a valid value

In non-interactive mode, we shouldn't try to prompt for an intro
description.

In summary, I think we should:

a) deprecate --intro
b) allow skipping the intro prompt
c) don't prompt when non-interactive


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list