[Quilt-dev] Re: [PATCH] Don't 'quilt delete' an unrefreshed patch

[Don Mullis]

Merging in your tidy argument-forwarding code, I come up with the
following (applies against the upstream git tree).  Want to review it?

I confess I already have some doubts about the design tradeoff of
duplicating so much of the UI of `quilt pop' in `quilt delete'.  An
alternative would be to remove any ability to delete applied patches
from the command.  If the user really wants to be able to delete the
topmost applied patch, he can easily enough code up a private
"porcelain-layer" script to do something like:

    quilt pop [-fR] && quilt delete -n

Similar doubts strike me regarding Randy Dunlap's RFC
"quilt delete any_nonconflicting_patch".

---


Modify the functionality of `quilt delete' to exit with an error
if it detects unrefreshed changes.

To access the old functionality, a `quilt delete -f' option is
introduced.  Note that this brings the paradigm of
`quilt delete'/`quilt delete -f' into accord with that of
`quilt pop'/`quilt pop -f'.  For either command, nothing will be
done if there are unrefreshed patches, unless '-f' is specified.
The consistency eliminates a detail that the user formerly had to
remember.

There is a potential to break scripts that might have unwittingly relied
on the old behavior.  A backward-compatible alternative is possible,
but would complicate the UI.

To help motivate this UI change, consider the following misadventure
that arose in a real-life interactive quilt session of an actual user
(who wishes to remain anonymous).

Suppose you're working on a patch p1, then realize that some changes
you're about to add really belong in a separate patch.  So you do

        <save buffers and kill editor with p1 changes>
        quilt new p2
        <add files to p2 and edit them>
        quilt refresh
        quilt pop

Looking again at p1, you realize that it too belongs somewhere else in
the stack, so as the first step in moving it you do ...

         quilt delete

... and all your unrefreshed changes to p1 are lost, irretrievably,
because there was no `quilt refresh' of p1 along the way.
---
 quilt/delete.in  |   13 +++++++++++--
 test/delete.test |   47 ++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 53 insertions(+), 7 deletions(-)

Index: quilt/test/delete.test
===================================================================
--- quilt.orig/test/delete.test 2010-01-03 21:11:05.000000000 -0800
+++ quilt/test/delete.test      2010-01-03 21:15:05.000000000 -0800
@@ -77,9 +77,11 @@ Test the delete command.
        $ touch .pc/test3/dir/file
        $ chmod a-rx .pc/test3/dir
 
-       $ quilt delete "test3"
-       > Removing patch %{P}test3
-       > .pc/test3/dir: Permission denied
+       # `quilt delete' does not specify -f to `quilt pop' now, and the
+       # the latter writes an absolute path into its message on stderr.
+       # Deal with this by throwing stderr away.
+       $ quilt delete "test3" 2>/dev/null || echo "delete failed as expected"
+       > delete failed as expected
 
        $ chmod a+rx .pc/test3/dir
 
@@ -89,16 +91,51 @@ Test the delete command.
        > .pc/test3/dir/file
 
        $ quilt applied
-       > No patches applied
+       > patches/test3
 
        $ quilt series
        > %{P}test3
 
        $ quilt delete
+       > Removing patch patches/test3
        > No patches applied
+       > Removed patch patches/test3
 
        $ quilt delete test3
-       > Removed patch %{P}test3
+       > No patches in series
+
+       $ quilt series
+
+       # Do not allow deletion of an unrefreshed patch
+       $ quilt new test4
+       > Patch %{P}test4 is now on top
+       $ quilt add file
+       > File file added to patch patches/test4
+       $ echo "x" >file
+       $ quilt delete
+       > Patch %{P}test4 does not remove cleanly (refresh it or enforce with 
-f)
+       $ quilt refresh
+       > Refreshed patch %{P}test4
+
+       $ quilt delete
+       > Removing patch patches/test4
+       > No patches applied
+       > Removed patch %{P}test4
+
+       $ quilt series
+
+       # Do not allow deletion of an unrefreshed patch, unless "-f" is 
specified
+       $ quilt new test4
+       > Patch %{P}test4 is now on top
+       $ quilt add file
+       > File file added to patch patches/test4
+       $ echo "x" >file
+       $ quilt delete -f
+       > Removing patch %{P}test4
+       > No patches applied
+       > Removed patch %{P}test4
 
        $ cd ..
        $ rm -rf d
+
+
Index: quilt/quilt/delete.in
===================================================================
--- quilt.orig/quilt/delete.in  2010-01-03 21:21:50.000000000 -0800
+++ quilt/quilt/delete.in       2010-01-03 21:22:06.000000000 -0800
@@ -35,6 +35,12 @@ topmost patch can be removed right now.)
 --backup
        Rename the patch file to patch~ rather than deleting it.
        Ignored if not used with \`-r'.
+
+-f     Force remove. If patch was applied, the previous state will
+       be restored from backup files, and any unrefreshed changes lost.
+
+-R     Always verify if the patch is refreshed; don't rely on timestamp
+       checks.
 "
 
                exit 0
@@ -43,7 +49,7 @@ topmost patch can be removed right now.)
        fi
 }
 
-options=`getopt -o nrh --long backup -- "$@"`
+options=`getopt -o nrhfR --long backup -- "$@"`
 
 if [ $? -ne 0 ]
 then
@@ -66,6 +72,9 @@ do
        --backup)
                QUILT_BACKUP=1
                shift ;;
+       -f | -R)
+               opt_pop="${opt_pop+$opt_pop }$1"
+               shift ;;
        --)
                shift
                break ;;
@@ -103,7 +112,7 @@ if is_applied "$patch"; then
                       "$(print_patch "$patch")" >&2
                exit 1
        fi
-       if ! quilt_command pop -fq
+       if ! quilt_command pop -q $opt_pop
        then
                exit 1
        fi