Re: [StGIT PATCH 1/5] Allow stack.patchorder.all to return hidden patches

["Karl Hasselström" <kha@treskal.com>]

On 2008-06-04 22:13:17 +0100, Catalin Marinas wrote:

> A new property, patchorder.all_visible, was added to return only the
> applied + unapplied patches. This is used in the "commit" command to
> avoid automatically committing the hidden patches.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

Acked-by: Karl Hasselström <kha@treskal.com>

Provided you improve the error message (see below).

> diff --git a/stgit/commands/commit.py b/stgit/commands/commit.py
> index cc2f13a..1bdbeea 100644
> --- a/stgit/commands/commit.py
> +++ b/stgit/commands/commit.py
> @@ -45,11 +45,11 @@ options = [make_option('-n', '--number', type = 'int',
>  def func(parser, options, args):
>      """Commit a number of patches."""
>      stack = directory.repository.current_stack
> -    args = common.parse_patches(args, list(stack.patchorder.all))
> +    args = common.parse_patches(args, list(stack.patchorder.all_visible))
>      if len([x for x in [args, options.number != None, options.all] if x]) > 1:
>          parser.error('too many options')
>      if args:
> -        patches = [pn for pn in stack.patchorder.all if pn in args]
> +        patches = [pn for pn in stack.patchorder.all_visible if pn in args]
>          bad = set(args) - set(patches)
>          if bad:
>              raise common.CmdException('Bad patch names: %s'

Forbidding commit to operate on hidden patches? OK, why not? But
saying "Bad patch name" if the user gives a hidden patch is probably a
bit on the grumpy side ... :-)

> diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
> index f9e750e..bdd21b1 100644
> --- a/stgit/lib/stack.py
> +++ b/stgit/lib/stack.py
> @@ -102,8 +102,8 @@ class PatchOrder(object):
>                           lambda self, val: self.__set_list('unapplied', val))
>      hidden = property(lambda self: self.__get_list('hidden'),
>                        lambda self, val: self.__set_list('hidden', val))
> -    # don't return the hidden patches, these have to be returned explicitly
> -    all = property(lambda self: self.applied + self.unapplied)
> +    all = property(lambda self: self.applied + self.unapplied + self.hidden)
> +    all_visible = property(lambda self: self.applied + self.unapplied)
>  
>  class Patches(object):
>      """Creates L{Patch} objects. Makes sure there is only one such object

Nice compromise to the debate on whether "all" should include hidden
patches.

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