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

[Mads Kiilerich]

On 07/06/2010 09:34 AM, Gilles Moris wrote:
> 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()

pn = rpn.split('#', 1)[0].rstrip()
might be more legible.

>      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 ?

That's just to make sure it works with both empty lines and comments. 
That's the extremes, so if both works there are pretty good odds that 
everything works.

>> +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.

Will you also include this test case with your fix?

/Mads