Bug #31664 for XML-Twig: keep_atts_order not working when using XML::Twig::Elt::copy

Tue Dec 18 15:27:33 2007 warrenbrown.5000 [...] yahoo.com - Ticket created

Subject:

keep_atts_order not working when using XML::Twig::Elt::copy

XML::Twig-3.17 (and beyond) perl v5.6.1, v5.8.4 Element attributes are not preserving their order when using elt->copy. For me, this change mentioned below in XML::Twig::Elt::copy fixed the problem: sub copy { . . . # original code commented out # #{ my %atts= %{$atts}; # we want to do a real copy of the attributes # $copy->set_atts( \%atts); #} { my %atts; tie %atts, 'Tie::IxHash' if (keep_atts_order()); %atts = %{$atts}; # we want to do a real copy of the attributes $copy->set_atts( \%atts); } . . . }

Tue Dec 18 16:17:11 2007 xmltwig [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #31664] keep_atts_order not working when using XML::Twig::Elt::copy
Date:	Tue, 18 Dec 2007 22:17:08 +0100
To:	bug-XML-Twig [...] rt.cpan.org
From:	mirod <xmltwig [...] gmail.com>

jbubbabrown via RT wrote: Show quoted text

> Element attributes are not preserving their order when using elt->copy. > > For me, this change mentioned below in XML::Twig::Elt::copy fixed the > problem: > > sub copy { > . > . > . > # original code commented out > # > #{ my %atts= %{$atts}; # we want to do a real copy of the attributes > # $copy->set_atts( \%atts); > #} > > { my %atts; > tie %atts, 'Tie::IxHash' if (keep_atts_order()); > %atts = %{$atts}; # we want to do a real copy of the attributes > $copy->set_atts( \%atts); > }

Hi, Nice catch. I'll add this to the next release. Thanks -- mirod

Tue Dec 18 16:17:24 2007 The RT System itself - Status changed from 'new' to 'open'

Wed Dec 31 14:11:16 2008 barries [...] slaysys.com - Correspondence added

A more succinct fix that works here is to modify the code to not do a copy, but to pass in the reference; set_atts() then copies from one IxHash to another. Here's the relevant snippet from a monkey patch that Works For Me, but I'm not sure why both copy() and set_atts() were copying the hash, could there be an overload of set_atts() that doesn't copy the hash? if( my $atts= $elt->atts) { $copy->set_atts($atts); } - Barrie On Tue Dec 18 16:17:11 2007, xmltwig@gmail.com wrote: Show quoted text

> jbubbabrown via RT wrote: > >

> > Element attributes are not preserving their order when using elt->copy. > > > > For me, this change mentioned below in XML::Twig::Elt::copy fixed the > > problem: > > > > sub copy { > > . > > . > > . > > # original code commented out > > # > > #{ my %atts= %{$atts}; # we want to do a real copy of the attributes > > # $copy->set_atts( \%atts); > > #} > > > > { my %atts; > > tie %atts, 'Tie::IxHash' if (keep_atts_order()); > > %atts = %{$atts}; # we want to do a real copy of the attributes > > $copy->set_atts( \%atts); > > }

> > Hi, > > Nice catch. I'll add this to the next release. > > Thanks >

Thu Jan 01 07:33:59 2009 xmltwig [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #31664] keep_atts_order not working when using XML::Twig::Elt::copy
Date:	Thu, 1 Jan 2009 07:33:46 -0500
To:	bug-XML-Twig [...] rt.cpan.org
From:	"Michel Rodriguez" <xmltwig [...] gmail.com>

On Wed, Dec 31, 2008 at 2:11 PM, Barrie Slaymaker via RT <bug-XML-Twig@rt.cpan.org> wrote: Show quoted text

> A more succinct fix that works here is to modify the code to not > do a copy, but to pass in the reference; set_atts() then copies > from one IxHash to another. Here's the relevant snippet from a > monkey patch that Works For Me, but I'm not sure why both copy() > and set_atts() were copying the hash, could there be an overload of > set_atts() that doesn't copy the hash?

Thanks, I will check if this works, probably next week though. The copying of the hash in set_atts was added a while ago because I (and others IIRC) had been bitten a few times by attributes sneakily changing values when the hash pointed by the hashref was modified. So this one should stay. set_atts is not overloaded or inlined or anything. -- mirod

Mon Mar 02 05:50:00 2009 RMINNER [...] cpan.org - Correspondence added

Hello, attached a demo for the copy problem. And a .diff for Twig.pm which fixes the lost attribute order (Barries fix)and a fix for an additional problem i encountered. When I run the attached demo script i get the following output: STRING:<foo a="1" b="2" c="3">power<bar></bar></foo> PARSED:<foo a="1" b="2" c="3">power<bar></bar></foo> COPY: <foo c="3" a="1" b="2">power<bar/></foo> Not only the element order is lost, but also the empty bar element is changed to a condensed form. This isn't desired in my case, since it causes changes in the file even when the content did not change. The patch also includes a fix for this undesired changed to the short form. Regards Roland

use strict; use warnings; use XML::Twig; my $twig=XML::Twig->new('keep_encoding' => 1, 'keep_spaces' => 1 , 'keep_atts_order' => 1); my $string = q{<foo a="1" b="2" c="3">power<bar></bar></foo>}; my $doc = $twig->safe_parse($string); my $root = $doc->root(); my $copy = $root->copy(); print 'STRING:' , $string , "\n"; print 'PARSED:' , $root->sprint() , "\n"; print 'COPY: ' , $copy->sprint() , "\n";

7045c7045 < if( ($elt->{'empty'} || 0)) { $copy->{empty}= 1; } # do not swap or speedup will mess up this --- > if( exists $elt->{'empty'}) { $copy->{empty}= $elt->{empty}; } 7064,7065c7064,7065 < { my %atts= %{$atts}; # we want to do a real copy of the attributes < $copy->set_atts( \%atts); --- > { > $copy->set_atts( $atts);

Mon Feb 28 13:06:34 2011 MIROD [...] cpan.org - Status changed from 'open' to 'resolved'