Re: [Quilt-dev] [PATCH] give more meaningful error messages

[Jean Delvare]

Le lundi 25 janvier 2010 16:52, Jean Delvare a écrit :
> Le lundi 25 janvier 2010 11:45, Andreas Gruenbacher a écrit :
> > On Monday 25 January 2010 10:36:54 Jean Delvare wrote:
> > > What is the purpose of this change? What is the user going to do
> > > differently after that?
> > 
> > The improved message makes things less confusing, no?  I don't see a 
> > problem 
> > with this change ...
> 
> The "problems" I see are: increase in code size, presumably comming
> at the price of a small performance hit ("quilt push" seems to be 1%
> slower with this change), and possible inconsistencies in error
> message wording given the "no preliminary setup required" nature of
> quilt. Starting to describe differences which so far were supposed to
> be transparent to the user seems strange.
> 
> If we are going to apply this change still, then please consider
> moving the test to after "if [ -z "$patch" ]", to only slow down the
> error path and not the common path. A simple "if [ -e $SERIES ]"
> should do if I'm not mistaken. And please also update the test suite
> so that it doesn't fail after applying the patch.

Would the following be OK for all of you? This is a less intrusive
fix than the original proposal, with no performance hit in the
general case (series file exists and command succeeds.)

* * * * *

Differentiate output when $QUILT_PATCHES/series is not found from
when $QUILT_PATCHES/series is empty.
This has originally been reported as
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=557621

---
 quilt/scripts/patchfns.in |   14 ++++++++++++--
 test/delete.test          |    2 +-
 test/three.test           |   18 +++++++++---------
 3 files changed, 22 insertions(+), 12 deletions(-)

--- a/quilt/scripts/patchfns.in
+++ b/quilt/scripts/patchfns.in
@@ -490,7 +490,12 @@ find_first_patch()
        local patch=$(cat_series | head -n 1)
        if [ -z "$patch" ]
        then
-               printf $"No patches in series\n" >&2
+               if [ -e "$SERIES" ]
+               then
+                       printf $"No patches in series\n" >&2
+               else
+                       printf $"No series file found\n" >&2
+               fi
                return 1
        fi
 
@@ -502,7 +507,12 @@ find_last_patch()
        local patch=$(cat_series | tail -n 1)
        if [ -z "$patch" ]
        then
-               printf $"No patches in series\n" >&2
+               if [ -e "$SERIES" ]
+               then
+                       printf $"No patches in series\n" >&2
+               else
+                       printf $"No series file found\n" >&2
+               fi
                return 1
        fi
 
--- a/test/delete.test
+++ b/test/delete.test
@@ -3,7 +3,7 @@ Test the delete command.
        $ mkdir patches
 
        $ quilt delete
-       > No patches in series
+       > No series file found
 
        $ cat > test.txt
        < Calling pci_match_id() would be more feasible.
--- a/test/three.test
+++ b/test/three.test
@@ -2,31 +2,31 @@
 
        $ touch e
        $ quilt annotate e
-       > No patches in series
+       > No series file found
 
        $ quilt top
-       > No patches in series
+       > No series file found
 
        $ quilt next
-       > No patches in series
+       > No series file found
 
        $ quilt previous
-       > No patches in series
+       > No series file found
 
        $ quilt unapplied
-       > No patches in series
+       > No series file found
 
        $ quilt applied
-       > No patches in series
+       > No series file found
 
        $ quilt next patch1
-       > No patches in series
+       > No series file found
 
        $ quilt push patch1
-       > No patches in series
+       > No series file found
 
        $ quilt push
-       > No patches in series
+       > No series file found
 
        $ quilt pop
        > No patch removed


-- 
Jean Delvare
Suse L3