Skip Menu |

This queue is for tickets about the XML-SAX-Writer CPAN distribution.

Report information
The Basics
Id: 60504
Status: open
Priority: 0/
Queue: XML-SAX-Writer
People
Owner: Nobody in particular
Requestors: SILASMONK [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: Normal
Broken in: 0.53
Fixed in: (no value)

History Show all quoted text
  Thu Aug 19 04:21:58 2010 SILASMONK [...] cpan.org - Ticket created
Subject: various documentation issues and complaints about switch from Text::Iconv
1.) There are a number stray characters in the documentation of dubious encoding. (Documented in patch file). 2.) The implementation of the Encoder was changed from Text::Iconv to Encode without updating the documentation. (Documented in patch file).# 3.) Actually since this is an object oriented implementation what was the point of discontinuing support for Text::Iconv? What would it have cost you to continue maintaining it? 4.)In particular in the Debian packaging of 0.52 it was documented that how one could implement error handling using Text::Iconv. Could you put some thought into encoding error handling? 5.) Could we have proper pod documentation for each of the supported modules?
Subject: convert_errors.patch
Subject: Fix documentation issues * add more documentation on encoding errors * Implementation moved from Text::Iconv to Encode without updating documentation. Also the change broke Florian's suggestion. * Stripped a number of stray characters of dubious encoding. These appear to be blank in the code, appear as 'A' in the man page, and 'A^' in the CPAN web page. Author: Florian Ragwitz <rafl@debian.org> Last-Update: 2010-08-19 Reviewed-By: Nicholas Bamber <nicholas@periapt.co.uk> --- a/lib/XML/SAX/Writer.pm +++ b/lib/XML/SAX/Writer.pm @@ -437,16 +437,16 @@ =item * new(%hash) -This is the constructor for this object.  It takes a number of +This is the constructor for this object. It takes a number of parameters, all of which are optional. =item -- Output -This parameter can be one of several things.  If it is a simple +This parameter can be one of several things. If it is a simple scalar, it is interpreted as a filename which will be opened for -writing.  If it is a scalar reference, output will be appended to this -scalar.  If it is an array reference, output will be pushed onto this -array as it is generated.  If it is a filehandle, then output will be +writing. If it is a scalar reference, output will be appended to this +scalar. If it is an array reference, output will be pushed onto this +array as it is generated. If it is a filehandle, then output will be sent to this filehandle. Finally, it is possible to pass an object for this parameter, in which @@ -460,7 +460,7 @@ This should be a hash reference where the keys are characters sequences that should be escaped and the values are the escaped form -of the sequence.  By default, this module will escape the ampersand +of the sequence. By default, this module will escape the ampersand (&), less than (<), greater than (>), double quote ("), and apostrophe ('). Note that some browsers don't support the &apos; escape used for apostrophes so that you should be careful when outputting XHTML. @@ -482,7 +482,7 @@ =item -- EncodeTo -The character set encoding in which output should be encoded.  Again, +The character set encoding in which output should be encoded. Again, this defaults to UTF-8. =item -- QuoteCharacter @@ -588,10 +588,9 @@ =head1 THE ENCODER INTERFACE Encoders can be plugged in to allow one to use one's favourite encoder -object. Presently there are two encoders: Iconv and NullEncoder, and -one based on C<Encode> ought to be out soon. They need to implement -two methods, and may inherit from XML::SAX::Writer::NullConverter if -they wish to +object. Presently there are two encoders: Encode and NullEncoder. They +need to implement two methods, and may inherit from +XML::SAX::Writer::NullConverter if they wish to =over 4 @@ -605,6 +604,11 @@ =back +Note that the return value of the convert method is B<not> checked. Output may +be truncated if a character couldn't be converted correctly. To avoid problems +the encoder should take care encoding errors itself, for example by raising an +exception. + =head1 CUSTOM OUTPUT This module is generally used to write XML -- which it does most of the
  Thu Aug 19 04:25:45 2010 SILASMONK [...] cpan.org - Broken in 0.53 added
  Thu Aug 19 04:25:46 2010 SILASMONK [...] cpan.org - Fixed in 0.53 deleted
  Thu Aug 19 15:00:29 2010 cpan [...] prather.org - Correspondence added
On Thu Aug 19 04:21:58 2010, SILASMONK wrote: Show quoted text
> 1.) There are a number stray characters in the documentation of dubious > encoding. (Documented in patch file).
Your patch is applied. Thank you. Show quoted text
> 2.) The implementation of the Encoder was changed from Text::Iconv to > Encode without updating the documentation. (Documented in patch file).#
Actually I just missed that line in the documentation. I did remove the much larger paragraph talking about the IconV->Encode switch from the TODO section. I'm sorry for the confusion. Show quoted text
> 3.) Actually since this is an object oriented implementation what was > the point of discontinuing support for Text::Iconv? What would it have > cost you to continue maintaining it?
Moving from Text::Iconv -> Encode has been a TODO item for several years before I took over maintenance. The switch was provided via a patch submitted in https://rt.cpan.org/Ticket/Display.html?id=46479 which also explains the problems with Text::Iconv on Win32 systems. This switch had been planned for at least a half decade, and it was causing enough pain for someone to submit a patch that switched. Ultimately I inherited this module simply to fix another TODO item (the ability to change the attribute quoting character), the cost to me would have involved re-writing a contributed patch to support a feature I didn't know people were relying upon. I simply don't have the time or money to pay for that kind of pre-cognition. Show quoted text
> 4.)In particular in the Debian packaging of 0.52 it was documented that > how one could implement error handling using Text::Iconv. Could you put > some thought into encoding error handling?
I don't use Debian when given a choice. I never use the system packaged CPAN distributions. I'm not aware of this documentation. Could someone perhaps have contributed it back upstream? Show quoted text
> 5.) Could we have proper pod documentation for each of the supported > modules?
The patch for Iconv->Encode was supplied, it looked reasonable and fixed a particularly painful issue for a class of users. I'm more than happy to apply such a patch from you or someone else, but I am regretfully unable at this time to devote the resources required to write such documentation myself. -Chris
  Thu Aug 19 15:00:30 2010 The RT System itself - Status changed from 'new' to 'open'
  Thu Aug 19 15:00:30 2010 cpan [...] prather.org - Status changed from 'open' to 'patched'
  Thu Aug 19 16:56:56 2010 nicholas [...] periapt.co.uk - Correspondence added
Subject: Re: [rt.cpan.org #60504] various documentation issues and complaints about switch from Text::Iconv
Date: Thu, 19 Aug 2010 21:56:45 +0100
To: bug-XML-SAX-Writer [...] rt.cpan.org
From: nicholas [...] periapt.co.uk
Chris, Your point about why these patches were not applied upstream earlier is spot on. I recently got involved in the Debian perl group for reasons that have nothing to do with XML::SAX::Writer. Basically I decided that the Debian packaging system is a better way of managing a perl installation than CPAN. Also I wanted to fix something that depended on a lot of perl modules. Now once you get involved in Debian you hit the problem finding people who can have the permissions to actually upload stuff and the best way turns out to be to join one of the packaging teams. I came across XML::SAX::Writer simply as a module in my area of interest that has a new upstream version. Now the previous version had I think 6 patches. Two I have been able to simply delete as they are redundant. The next was referring to the old Text::Iconv code and so had to be rewritten which is what I have forwarded to you. There are two more that should really have been forwarded upstream. However they are somewhat more non-trivial. I was thinking that I ought to do some assessment of them before sending them upstream but I don;t really have time for that at the moment - may be in six months. If you are willing I could send you those patches and you could assess them. I could do it either via RT or directly. I would prefer RT as it would keep the Debian "paperwork" cleaner. Anyway thanks for your response. Nicholas Quoting Chris Prather via RT <bug-XML-SAX-Writer@rt.cpan.org>: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=60504 > > > On Thu Aug 19 04:21:58 2010, SILASMONK wrote:
>> 1.) There are a number stray characters in the documentation of dubious >> encoding. (Documented in patch file).
> > Your patch is applied. Thank you. >
>> 2.) The implementation of the Encoder was changed from Text::Iconv to >> Encode without updating the documentation. (Documented in patch file).#
> > Actually I just missed that line in the documentation. I did remove > the much larger paragraph > talking about the IconV->Encode switch from the TODO section. I'm > sorry for the confusion. >
>> 3.) Actually since this is an object oriented implementation what was >> the point of discontinuing support for Text::Iconv? What would it have >> cost you to continue maintaining it?
> > Moving from Text::Iconv -> Encode has been a TODO item for several > years before I took > over maintenance. The switch was provided via a patch submitted in > https://rt.cpan.org/Ticket/Display.html?id=46479 which also explains > the problems with > Text::Iconv on Win32 systems. This switch had been planned for at > least a half decade, and it > was causing enough pain for someone to submit a patch that switched. > > Ultimately I inherited this module simply to fix another TODO item > (the ability to change the > attribute quoting character), the cost to me would have involved > re-writing a contributed > patch to support a feature I didn't know people were relying upon. I > simply don't have the > time or money to pay for that kind of pre-cognition. >
>> 4.)In particular in the Debian packaging of 0.52 it was documented that >> how one could implement error handling using Text::Iconv. Could you put >> some thought into encoding error handling?
> > I don't use Debian when given a choice. I never use the system > packaged CPAN distributions. > I'm not aware of this documentation. Could someone perhaps have > contributed it back > upstream? >
>> 5.) Could we have proper pod documentation for each of the supported >> modules?
> > The patch for Iconv->Encode was supplied, it looked reasonable and > fixed a particularly > painful issue for a class of users. I'm more than happy to apply > such a patch from you or > someone else, but I am regretfully unable at this time to devote the > resources required to > write such documentation myself. > > -Chris >
  Thu Aug 19 16:56:57 2010 The RT System itself - Status changed from 'patched' to 'open'