Re: [StGit PATCH 5/9] Convert 'hide' to the lib infrastructure

From: Karl Hasselström <kha@treskal.com>
Date: Apr 29, 2009, 06:27 UTC
Message-ID: <20090429062749.GE18483@diana.vm.bytemark.co.uk>
In-Reply-To: <20090428151003.27261.72531.stgit@pc1117.cambridge.arm.com>

On 2009-04-28 16:10:03 +0100, Catalin Marinas wrote:

> -args = [argparse.patch_range(argparse.applied_patches,
> -                             argparse.unapplied_patches)]
> +args = [argparse.patch_range(argparse.unapplied_patches)]

Why not simply allow all patches? reorder_patches() below will happily pop unapplied patches before hiding them IIRC, and for already hidden patches you could just say "already hidden".

Hmm, but this is for the tab completion, so I guess we'd only want the applied and unapplied patches here.

Show 13 quoted lines

> +    patches = common.parse_patches(args, trans.all_patches)
> +    for p in patches:
> +        if p in trans.applied:
> +            raise common.CmdException('Cannot hide applied patch "%s"' % p)
> +        elif p in trans.hidden:
> +            raise common.CmdException('Patch "%s" already hidden' % p)
> +
> +    applied = list(trans.applied)
> +    unapplied = [p for p in trans.unapplied if not p in set(patches)]
> +    hidden = patches + trans.hidden
> +
> +    trans.reorder_patches(applied, unapplied, hidden)
> +    return trans.run()

As I said,

  * Why not simply allow hiding of applied patches?

  * Hiding a hidden patch should probably be a warning (if that), not
    an error.

Otherwise it looks good.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle