[PATCH 2 of 3] mq: refactor mq.patchheader

[Patrick Mézard]

Le 13/02/10 02:11, Steve Losh a écrit :
> # HG changeset patch
> # User Steve Losh <steve at stevelosh.com>
> # Date 1265763661 18000
> # Node ID 3c1eee3d90378bde9502bb2f7098f97bc67be6a9
> # Parent  60564b40a8f6989fdb1b2997e4ce5334ffc4544c
> mq: refactor mq.patchheader
> 
> This patch refactors the patchheader class in mq.py to be more understandable
> and more robust when working with plain- and hg-style patch headers.
> 
> Some of the tests have also been updated to reflect the new, more consistent
> ordering of the header fields.

The code structure is much better now.

But I would question the parser rewrite. The new one does not behave like the previous one in the following ways:

1- The line/message stripping behaviour differs. The current parser, rstrip() all lines, *including* the non message lines. The new one strip() the whole header, does not strip() the non-message lines, then strip() again the message itself. I think stripping leading whitespace on the message is incorrect, we never do that, and not rstripping() non-message lines may be fragile for no good reason.

2- The new parser understands a subset of the non-message headers handled by the current one:
  a) The current parser adds all non-blank lines and non-meta lines to the message, whether they come before or after the meta lines. The new one looks for meta lines first, then adds what follows to the message. I think this is better than the previous behaviour. Except I would be a bit more tolerant in the from/date case and ignore unknown lines until a blank line is reached.
  b) The new parser ignores the "Subject: " header. I have no opinion on this.

3- I am not fond of the while/pop() style. I usually avoid touching the input stream and my brain refuses to pop(0) from basic resizable arrays. But that should be no problem here and unless everybody feel like me, this could stay.


I think [1] should be fixed.

I'd like to hear Matt on [2].

If [2] is rejected, the parser should probably be kept as-is while dropping all the getter/setter stuff and replacing them with the light serialization methods.

--
Patrick Mézard