[PATCH stable] qpush --move: move the right patch even with comment lines

[Gilles Moris]

On Friday 02 July 2010 03:33:37 pm Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <mads at kiilerich.com>
> # Date 1278077561 -7200
> # Branch stable
> # Node ID 84e9c15edee60cab95b3c635dd257d8b49226237
> # Parent  239f3210c970615dc1d5f861a92b61b4662a71a5
> qpush --move: move the right patch even with comment lines
>
> 88fc876a4833 caused that we find the index of the moving patch in
> self.series but look it up in self.full_series. The difference between
> these is that full_series also contains comment lines, and we thus moved
> the wrong patch.
>

Yes, that was intended because full_series also contains qguards in the form 
of comments appended to the patch name. But I did not account for regular 
comments.

> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -1047,7 +1047,7 @@
>
>              if move:
>                  try:
> -                    index = self.series.index(patch, start)
> +                    index = self.full_series.index(patch, start)
>                      fullpatch = self.full_series[index]
>                      del self.full_series[index]
>                  except ValueError:

Yes, this will break my 88fc876a4833 fix for qguard, so I will have to work 
only with full_series and be much smarter to find matches. I can probably 
*not* use the the startswith() method because of patches that start with the 
same root name. I am thinking about something like:

for i, rpn in enumerate(self.full_series[start:]):
    try:
        # find markers for patch guards
        pn = rpn[:rpn.index('#')].rstrip()
    except ValueError:
        pn = rpn
    if pn == patch:
        index = i + start
        break

I don't know if we should be even smarter for the case were '#' is really in 
the patch name, so we could for instance match the regexp ' #[+-]' to find 
everything that really looks like a positive or negative guard.

> diff --git a/tests/test-mq b/tests/test-mq
> --- a/tests/test-mq
> +++ b/tests/test-mq
> @@ -104,6 +104,11 @@
>
>  hg qnew -m 'foo bar' test.patch
>
> +echo '# comment' > .hg/patches/series.tmp
> +echo >> .hg/patches/series.tmp

Does this empty line in series serve a special purpose here ?

> +cat .hg/patches/series >> .hg/patches/series.tmp
> +mv .hg/patches/series.tmp .hg/patches/series
> +
>  echo % qrefresh
>
>  echo a >> a
> @@ -236,6 +241,9 @@
>  hg qpush --move test.patch # already applied
>  hg qpush
>
> +echo % series after move
> +cat `hg root`/.hg/patches/series
> +
>  echo % pop, qapplied, qunapplied
>  hg qseries -v
>  echo % qapplied -1 test.patch
> diff --git a/tests/test-mq.out b/tests/test-mq.out
> --- a/tests/test-mq.out
> +++ b/tests/test-mq.out
> @@ -225,6 +225,12 @@
>  abort: cannot push to a previous patch: test.patch
>  applying test2.patch
>  now at: test2.patch
> +% series after move
> +test.patch
> +test1b.patch
> +test2.patch
> +# comment
> +
>  % pop, qapplied, qunapplied
>  0 A test.patch
>  1 A test1b.patch

Yes, I will have to add a test case for my qguard/sqelect workflow also.

Regards.
Gilles.