Bug #88501 for Tree-DAG_Node: use open qw(:std :utf8) ?

Sun Sep 08 15:09:19 2013 TEAM [...] cpan.org - Ticket created

Subject:

use open qw(:std :utf8) ?

Hi there, First off, thanks for the module - it has been very useful in avoiding the tedious parts of tree management =) Just upgraded to 1.14, and it appears that an 'open' directive has been added since the last version I was using (I think it was 1.12, although that doesn't appear to be on CPAN any more so I haven't reinstalled it to verify). This line: use open qw(:std :utf8); # Undeclared streams in UTF-8. sets the encoding for STDIN/STDOUT, which has global effect. If Tree::DAG_Node is being used to maintain the data structure but code elsewhere is handling the rendering, this breaks - see the attached images for an example. Forcing the encoding on STDIN/STDOUT doesn't seem like the right to do in this module. Maybe I don't fully understand the reasons behind it, but if this is part of the utf8 support for reading files then the usual approach is to leave the output alone (the user of the module is responsible for setting the correct encoding layer on the output handles) and perhaps offer an encoding option to select utf8/etc. for incoming file data? cheers, Tom

Subject:

2013-09-08-tree-dag_node-after.png

Download 2013-09-08-tree-dag_node-after.png
image/png 11.3k

Subject:

2013-09-08-tree-dag_node-before.png

Download 2013-09-08-tree-dag_node-before.png
image/png 6.3k

Sun Sep 08 18:32:06 2013 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #88501] use open qw(:std :utf8) ?
Date:	Mon, 09 Sep 2013 08:31:33 +1000
To:	bug-Tree-DAG_Node [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Tom On 09/09/13 05:09, Thomas Edward Alexander Molesworth via RT wrote: Show quoted text

> Sun Sep 08 15:09:19 2013: Request 88501 was acted upon. > Transaction: Ticket created by TEAM > Queue: Tree-DAG_Node > Subject: use open qw(:std :utf8) ? > Broken in: 1.14, 1.15 > Severity: Important > Owner: Nobody > Requestors: TEAM@cpan.org > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=88501 > > > > Hi there, > > First off, thanks for the module - it has been very useful in avoiding the tedious parts of tree management =)

Thanx. Of course I'm only the maintainer.... Show quoted text

> Just upgraded to 1.14, and it appears that an 'open' directive has been added since the last version I was using (I think it was 1.12, although that doesn't appear to be on CPAN any more so I haven't reinstalled it to verify).

Yes, I added that. Let me think about the code. I was slightly uneasy when I added it. I'll see if I can limit it's effect with adding encoding to the open statement(s). Show quoted text

> This line: > > use open qw(:std :utf8); # Undeclared streams in UTF-8. > > sets the encoding for STDIN/STDOUT, which has global effect. If Tree::DAG_Node is being used to maintain the data structure but code elsewhere is handling the rendering, this breaks - see the attached images for an example. > > Forcing the encoding on STDIN/STDOUT doesn't seem like the right to do in this module. Maybe I don't fully understand the reasons behind it, but if this is part of the utf8 support for reading files then the usual approach is to leave the output alone (the user of the module is responsible for setting the correct encoding layer on the output handles) and perhaps offer an encoding option to select utf8/etc. for incoming file data? > > cheers, > > Tom >

-- Ron Savage http://savage.net.au/ Ph: 0421 920 622

Sun Sep 08 18:32:06 2013 The RT System itself - Status changed from 'new' to 'open'

Sun Sep 08 18:53:09 2013 TEAM [...] cpan.org - Correspondence added

Thanks for the quick response. Some notes below about the attached patch... On Sun Sep 08 18:32:06 2013, ron@savage.net.au wrote: Show quoted text

> Hi Tom > > On 09/09/13 05:09, Thomas Edward Alexander Molesworth via RT wrote:

> > Just upgraded to 1.14, and it appears that an 'open' directive has > > been added since the last version I was using (I think it was 1.12, > > although that doesn't appear to be on CPAN any more so I haven't > > reinstalled it to verify).

> > Yes, I added that. Let me think about the code. I was slightly uneasy > when I added it. I'll see if I can limit it's effect with adding > encoding to the open statement(s).

If it's any use, the patch attached does the following things: * removes use open * switches to File::Slurp with binmode option * updates the min perl version check in Makefile.PL which I'm using for my local installation (in decreasing order of relevance to this ticket). I don't know what the File::Slurp issues mentioned in the documentation were, but the tests are passing for me with those changes in place, and it seems happy enough with UTF8 data via read_tree or when using the API. Dumping the tree needs binmode STDOUT, ':encoding(UTF-8)';, of course, but that would apply to any Unicode output (use utf8; print "some unicode string" for example). cheers, Tom

Subject:

Tree-DAG_Node-1.15-patch

Download Tree-DAG_Node-1.15-patch
application/octet-stream 6k

Message body not shown because it is not plain text.

Sun Sep 08 20:09:32 2013 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #88501] use open qw(:std :utf8) ?
Date:	Mon, 09 Sep 2013 10:09:14 +1000
To:	bug-Tree-DAG_Node [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Tom (1) I have accepted your very nice patch, with tiny mods. $many x $thanx; V 1.16 is on CPAN. (2) Patch patches: I advocate all Perlers use 'utf8' rather than 'utf-8'. It's just 1 more thing to confuse beginners. I'm aware of this 'design fault': <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> but I do what I can :-). (3) I adopted the 'use open qw(:std :utf8);' stuff because of this advice: http://www.perl.com/pub/2012/04/perlunicook-standard-preamble.html (4) I overlooked File::Slurp's binmode option, thinking only of Perl's binmode. (5) My policy is to remove the previous version from CPAN, unless I have some doubts about the new version. Hence your difficulty in finding same. I am aware this might discompose some users, and am trying to retrain myself to leave a version of 2 there for a while. Cheers -- Ron Savage http://savage.net.au/ Ph: 0421 920 622

Sun Sep 08 20:21:07 2013 TEAM [...] cpan.org - Correspondence added

Hi Ron, On Sun Sep 08 20:09:32 2013, ron@savage.net.au wrote: Show quoted text

> Hi Tom > > (1) > I have accepted your very nice patch, with tiny mods. > > $many x $thanx; > > V 1.16 is on CPAN.

That was quick indeed - I'll give it a shot now. Show quoted text

> (2) > Patch patches: I advocate all Perlers use 'utf8' rather than 'utf-8'. > It's just 1 more thing to confuse beginners. > > I'm aware of this 'design fault': > > <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> > > but I do what I can :-).

I'm happy with that - my main reason for UTF-8 over utf8 is that 'utf8' isn't as strict. perldoc Encode has more details in the "UTF-8 vs. utf8 vs. UTF8" section: As of Perl 5.8.7, "UTF-8" means UTF-8 in its current sense, which is conservative and strict and security-conscious, whereas "utf8" means UTF-8 in its former sense, which was liberal and loose and lax. The FATAL => utf8 warning probably covers this already though. Show quoted text

> (3) > I adopted the 'use open qw(:std :utf8);' stuff because of this advice: > > http://www.perl.com/pub/2012/04/perlunicook-standard-preamble.html

Ah, I see. That probably makes more sense for a script than a module. Show quoted text

> (5) > My policy is to remove the previous version from CPAN, unless I have > some doubts about the new version. Hence your difficulty in finding > same. I am aware this might discompose some users, and am trying to > retrain myself to leave a version of 2 there for a while.

That's fine - there's always backpan if necessary, I'm probably guilty of leaving too many old versions around =) Will shout if there are any problems with the new version, but for now I think everything's looking good if you want to close the ticket. cheers, Tom

Sun Sep 08 20:43:46 2013 RSAVAGE [...] cpan.org - Correspondence added

Patched for V 1.16 as discussed in messages.

Sun Sep 08 20:43:47 2013 RSAVAGE [...] cpan.org - Status changed from 'open' to 'resolved'

Fri Jul 10 14:32:23 2015 TEAM [...] cpan.org - Reference by ticket #105798 added