Teach "stg delete" to delete several patches at once
cover Teach "stg delete" to delete several patches at once
The first patch introduces a test for "stg delete", since there wasn't already one, and it's much less stressful to modify stuff if you have an automatic test that can tell you if you broke stuff.
As it turns out, this test finds an existing bug: it's not possible to delete a patch in another branch if there's an identically named branch in the current branch, and both patches are topmost in their respective applied stacks.
The second patch teaches "delete" how to delete multiple patches in one go.
Signature · 2 lines
Karl Hasselström, [email protected] www.treskal.com/kalle
PATCH 1/2 Regression test for "stg delete"
From: Karl Hasselström <[email protected]>
NOTE: The subtest that attempts to delete a patch in another branch currently fails, because there's a bug in "delete".
---
t/t1600-delete.sh | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 109 insertions(+), 0 deletions(-)
t/t1600-delete.sh new file +109 -0
@@ -0,0 +1,109 @@ +#!/bin/sh +# Copyright (c) 2006 Karl Hasselström +test_description='Test the delete command.' +. ./test-lib.sh + +test_expect_success \ + 'Initialize the StGIT repository' \ + 'stg init' + +test_expect_success \ + 'Create a patch' \ + ' + stg new foo -m foo && + echo foo > foo.txt && + stg add foo.txt && + stg refresh + ' + +test_expect_success \ + 'Try to delete a non-existing patch' \ + ' + [ $(stg applied | wc -l) -eq 1 ] && + ! stg delete bar && + [ $(stg applied | wc -l) -eq 1 ] + ' + +test_expect_success \ + 'Try to delete the topmost patch while dirty' \ + ' + echo dirty >> foo.txt && + [ $(stg applied | wc -l) -eq 1 ] && + ! stg delete foo && + [ $(stg applied | wc -l) -eq 1 ] && + git reset --hard + ' + +test_expect_success \ + 'Delete the topmost patch' \ + ' + [ $(stg applied | wc -l) -eq 1 ] && + stg delete foo && + [ $(stg applied | wc -l) -eq 0 ] + ' + +test_expect_success \ + 'Create an unapplied patch' \ + ' + stg new foo -m foo && + echo foo > foo.txt && + stg add foo.txt && + stg refresh && + stg pop + ' + +test_expect_success \ + 'Delete an unapplied patch' \ + ' + [ $(stg unapplied | wc -l) -eq 1 ] && + stg delete foo && + [ $(stg unapplied | wc -l) -eq 0 ] + ' + +test_expect_success \ + 'Create two patches' \ + ' + stg new foo -m foo && + echo foo > foo.txt && + stg add foo.txt && + stg refresh && + stg new bar -m bar && + echo bar > bar.txt && + stg add bar.txt && + stg refresh + ' + +test_expect_success \ + 'Try to delete a non-topmost applied patch' \ + ' + [ $(stg applied | wc -l) -eq 2 ] && + ! stg delete foo && + [ $(stg applied | wc -l) -eq 2 ] + ' + +test_expect_success \ + 'Create another branch, and put one patch in each branch' \ + ' + stg branch --create br && + stg new baz -m baz && + echo baz > baz.txt && + stg add baz.txt && + stg refresh && + stg branch master && + stg new baz -m baz && + echo baz > baz.txt && + stg add baz.txt && + stg refresh + ' + +test_expect_success \ + 'Delete a patch in another branch' \ + ' + [ $(stg applied | wc -l) -eq 3 ] && + [ $(stg applied -b br | wc -l) -eq 1 ] && + stg delete -b br baz && + [ $(stg applied | wc -l) -eq 3 ] && + [ $(stg applied -b br | wc -l) -eq 0 ] + ' + +test_done
PATCH 2/2 Allow deletion of several patches at once
In reply to Karl HasselströmFrom: Karl Hasselström <[email protected]>
---
stgit/commands/delete.py | 50 ++++++++++++++++----- t/t1600-delete-one.sh | 109 ++++++++++++++++++++++++++++++++++++++++++++++ t/t1600-delete.sh | 109 ---------------------------------------------- t/t1601-delete-many.sh | 45 +++++++++++++++++++ 4 files changed, 193 insertions(+), 120 deletions(-)
stgit/commands/delete.py +39 -11
@@ -24,13 +24,16 @@ from stgit.utils import * from stgit import stack, git -help = 'remove the topmost or any unapplied patch' -usage = """%prog [options] <patch> +help = 'delete patches' +usage = """%prog [options] <patch> [ <patch> [...] ] -Delete the patch passed as argument. The patch to be deleted can only -be part of the unapplied list or be the topmost one, in the latter -case the command also popping it from the stack. Note that the -'delete' operation is irreversible.""" +Delete the patches passed as arguments. If an applied patch is to be +deleted, all other patches applied on top of it must be deleted too; +and they must be explicitly specified, since this command will not try +to delete a patch unless you explicitly ask it to. If any applied +patches are deleted, they are popped from the stack. + +Note that the 'delete' operation is irreversible.""" options = [make_option('-b', '--branch', help = 'use BRANCH instead of the default one')]
@@ -38,16 +41,41 @@ options = [make_option('-b', '--branch', def func(parser, options, args): """Deletes a patch """ - if len(args) != 1: - parser.error('incorrect number of arguments') + if not args: + parser.error('no patches to delete') + + applied = {} + unapplied = {} + dontexist = {} + for patch in args: + if patch in crt_series.get_unapplied(): + unapplied[patch] = None + elif patch in crt_series.get_applied(): + applied[patch] = None + else: + dontexist[patch] = None - if args[0] == crt_series.get_current(): + while crt_series.get_current() in applied: + patch = crt_series.get_current() check_local_changes() check_conflicts() check_head_top_equal() + crt_series.delete_patch(patch) + del applied[patch] + print 'Patch "%s" successfully deleted' % patch + + for patch in unapplied.iterkeys(): + crt_series.delete_patch(patch) + print 'Patch "%s" successfully deleted' % patch + + if dontexist: + print 'Error: no such patch: %s' % ', '.join(dontexist.iterkeys()) + if applied: + print 'Error: failed to delete %s' % ', '.join(applied.iterkeys()) - crt_series.delete_patch(args[0]) - print 'Patch "%s" successfully deleted' % args[0] + failed = len(dontexist) + len(applied) + if failed: + raise CmdException, 'Failed to delete %d patches' % failed if not options.branch: print_crt_patch()
t/t1600-delete-one.sh new file +109 -0
109 lines not on this page · raw message
t/t1600-delete.sh deleted +0 -109
109 lines not on this page · raw message
t/t1601-delete-many.sh new file +45 -0
45 lines not on this page · raw message
Re: [PATCH 2/2] Allow deletion of several patches at once
Quoted 2 lines · Karl Hasselström
t/t1600-delete-one.sh | 109 ++++++++++++++++++++++++++++++++++++++++++++++ t/t1600-delete.sh | 109 ----------------------------------------------
By the way, this is why I complained about StGIT's lack of rename support earlier. This is a file move with maybe three of 109 lines changed, but you won't be able to see that without importing the patch and examining it with the git tools. :-(
Signature · 2 lines
Karl Hasselström, [email protected] www.treskal.com/kalle
Re: [PATCH 2/2] Allow deletion of several patches at once
In reply to Karl Hasselströmstgit/commands/delete.py options = [make_option('-b', '--branch', · line 50
Quoted 4 lines · Karl Hasselström
stgit/commands/delete.py +4
-0+ applied = {} + unapplied = {} + dontexist = {} + for patch in args:
Any reason for not using the parse_patches() function in stgit.command.common? It allows one to specify patch ranges like patch1..patch2. Have a look at how it is used for the push or float commands.
Signature · 1 line
Catalin
Re: [PATCH 2/2] Allow deletion of several patches at once
Quoted 11 lines · Catalin Marinas
stgit/commands/delete.py options = [make_option('-b', '--branch', · line 50
Quoted 4 lines · Karl Hasselström
stgit/commands/delete.py +4
-0+ applied = {} + unapplied = {} + dontexist = {} + for patch in args:Any reason for not using the
parse_patches()function in stgit.command.common? It allows one to specify patch ranges like patch1..patch2. Have a look at how it is used for the push or float commands.
Ah, no, I just didn't know it existed. I'll send an updated patch (but it may be a few days).
Signature · 2 lines
Karl Hasselström, [email protected] www.treskal.com/kalle
PATCH 1/1 Allow deletion of several patches at once
From: Karl Hasselström <[email protected]>
---
This is an updated patch, which uses parse_patches() to support patch ranges. It replaces patch 2/2 in the series; patch 1/2 (original delete regression test) is unaffected.
stgit/commands/delete.py | 54 +++++++++++++++++------ t/t1600-delete-one.sh | 109 ++++++++++++++++++++++++++++++++++++++++++++++ t/t1600-delete.sh | 109 ---------------------------------------------- t/t1601-delete-many.sh | 55 +++++++++++++++++++++++ 4 files changed, 205 insertions(+), 122 deletions(-)
stgit/commands/delete.py +41 -13
@@ -24,30 +24,58 @@ from stgit.utils import * from stgit import stack, git -help = 'remove the topmost or any unapplied patch' -usage = """%prog [options] <patch> +help = 'delete patches' +usage = """%prog [options] [<patch1>] [<patch2>] [<patch3>..<patch4>] -Delete the patch passed as argument. The patch to be deleted can only -be part of the unapplied list or be the topmost one, in the latter -case the command also popping it from the stack. Note that the -'delete' operation is irreversible.""" +Delete the patches passed as arguments. If an applied patch is to be +deleted, all other patches applied on top of it must be deleted too; +and they must be explicitly specified, since this command will not try +to delete a patch unless you explicitly ask it to. If any applied +patches are deleted, they are popped from the stack. + +Note that the 'delete' operation is irreversible.""" options = [make_option('-b', '--branch', help = 'use BRANCH instead of the default one')] def func(parser, options, args): - """Deletes a patch - """ - if len(args) != 1: - parser.error('incorrect number of arguments') + """Deletes one or more patches.""" + applied_patches = crt_series.get_applied() + unapplied_patches = crt_series.get_unapplied() + all_patches = applied_patches + unapplied_patches + + if args: + patches = parse_patches(args, all_patches) + else: + parser.error('No patches specified') + + applied = {} + unapplied = {} + for patch in patches: + if patch in unapplied_patches: + unapplied[patch] = None + else: + applied[patch] = None - if args[0] == crt_series.get_current(): + while crt_series.get_current() in applied: + patch = crt_series.get_current() check_local_changes() check_conflicts() check_head_top_equal() + crt_series.delete_patch(patch) + del applied[patch] + print 'Patch "%s" successfully deleted' % patch + + for patch in unapplied.iterkeys(): + crt_series.delete_patch(patch) + print 'Patch "%s" successfully deleted' % patch + + if applied: + print 'Error: failed to delete %s' % ', '.join(applied.iterkeys()) - crt_series.delete_patch(args[0]) - print 'Patch "%s" successfully deleted' % args[0] + failed = len(applied) + if failed: + raise CmdException, 'Failed to delete %d patches' % failed if not options.branch: print_crt_patch()
t/t1600-delete-one.sh new file +109 -0
109 lines not on this page · raw message
t/t1600-delete.sh deleted +0 -109
109 lines not on this page · raw message
t/t1601-delete-many.sh new file +54 -0
54 lines not on this page · raw message
Re: [PATCH] Allow deletion of several patches at once
Quoted 3 lines · Karl Hasselström
This is an updated patch, which uses
parse_patches()to support patch ranges. It replaces patch 2/2 in the series; patch 1/2 (original delete regression test) is unaffected.
I applied it but added another commit to change the algorithm slightly. It now first checks whether the applied patches can be deleted before deleting any patch (I find this safer). It also checks for local changes, conflicts and HEAD == top only if you delete applied patches on the current branch. Actually, the HEAD == top test failed when deleting on a different branch, which is normal since the top is that of the other branch.
I'll make the changes public tonight.
Thanks.