Menu
▾
▴
Re: [Docutils-develop] Element.copy() to copy the rawsource
|
From: David G. <go...@py...> - 2010-07-02 13:56:17
|
On Fri, Jul 2, 2010 at 06:35, Georg Brandl <g.b...@gm...> wrote:
> Am 20.06.2010 15:52, schrieb David Goodger:
>> On Sun, Jun 20, 2010 at 06:36, Georg Brandl <g.b...@gm...> wrote:
>>> Any objections to this patch?
>>>
>>> Index: docutils/nodes.py
>>>
>>>
>>> ===================================================================
>>> --- docutils/nodes.py (Revision 6346)
>>> +++ docutils/nodes.py (Arbeitskopie)
>>> @@ -739,7 +739,7 @@
>>> for child in self.children])
>>>
>>> def copy(self):
>>> - return self.__class__(**self.attributes)
>>> + return self.__class__(self.rawsource, **self.attributes)
>>
>> No objection (assuming you run the test suite first, and it introduces
>> no bugs :-). I'd make it explicit though:
>>
>> return self.__class__(rawsource=self.rawsource, **self.attributes)
>>
>> That makes the code more self-documenting, and may help with
>> subclasses -- some of which don't implement the rawsource parameter.
>
> Hmm, in fact, it does fail the test suite because there is an explicit
> test that the rawsource of a deepcopy should be the empty string.
>
> There is also one more failure, where it the pseudoxml output of some
> translation is different because substitutions deepcopy their content.
>
> Is it ok to change these tests, or is the current behavior intended?
I don't recall why the tests are the way they are, other than to test
the then-expected behavior. IOW, the tests may simply have been a
side-effect of the code (the non-copying of rawsource).
The substitution test failure seems to be an improvement:
<problematic ids="id8" refid="id7">
+ |Sub|
Without the change, there's a <problematic> element with no content,
which could make diagnosis difficult if this were to occur in actual
use.
I can't think of any adverse effects of the proposed change, but
before "blessing" it: what is the purpose of the proposed change?
--
David Goodger <http://python.net/~goodger>
|