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

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.

+    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