[PATCH] mq: record more data in patchheader class

[Mads Kiilerich]

Steve Borho wrote, On 01/02/2011 11:21 PM:
> On Sun, Jan 2, 2011 at 3:55 PM, Mads Kiilerich<mads at kiilerich.com>  wrote:
>> Steve Borho wrote, On 01/01/2011 12:13 AM:
>>> # HG changeset patch
>>> # User Steve Borho<steve at borho.org>
>>> # Date 1293836978 21600
>>> # Node ID a28115e8e6e8cf8a4e2751898ca3dde8cc351d10
>>> # Parent  d85d8ad88de938f4a6a5a0e3818eab70fc0c2ec0
>>> mq: record more data in patchheader class
>>> Combined, these make mq.patchheader() very useful for parsing and
>>> preserving a patch header through edits.
>> Just to make it clear: will this actually change anything in Mercurial
>> (which then should be documented and tested?), or is it just something
>> TortoiseHg could benefit from?
> Existing Mercurial code will not use the new fields.
>
>>> * parse branch and nodeid header lines
>> Does this mean that patches will be pushed back to the branch they were
>> qref'ed/qpop'ed from? If so: Is that an acceptable change, and will we then
>> need some way of overruling this in order to be able to move patches from
>> one branch to another?
> No, it doesn't change any behavior except to remember the values it
> read from the header, just as patch.extract() does.  These aren't
> terribly important, it is just convenient to show them in the graph
> when the patches are unapplied.
>
>>> * remember the line number where diffs started
>> What is the rationale for this?
> This is for the new TortoiseHg shelve tool.  It uses mq.patchheader()
> parse patch headers and record.parsepatch() to parse the diff portion
> into selectable chunks.  To do this reliably, it needs to know the
> exact line on which the diff started.  mq.patchheader() discovers this
> value, then throws it away by consuming empty lines.  This one-line
> change to remember the original size of the patch comment before
> pruning empty lines is by far the most important part of the patch.

Thanks for the explanation.

It could be debated if we should expand the "internal API" just to help 
external users, but it seems to make sense in this case.

I think it deserves a comment in the code (and perhaps a longer 
changeset description with some the information you gave here) so future 
cleanups and refactorings doesn't remove this "redundant" code. At the 
same time it is an internal API, so we are free to change anything 
anyway ...

/Mads