Skip Menu |
Report information
Id: 131683
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: pali [at] cpan.org
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)

Attachments
0001-Encode-Add-new-check-flag-Encode-ONLY_PRAGMA_WARNING.patch
0002-PerlIO-encoding-Use-Encode-ONLY_PRAGMA_WARNINGS-in-f.patch



Date: Sat, 1 Jul 2017 13:08:13 +0200
From: pali [...] cpan.org
Subject: Encode::ONLY_PRAGMA_WARNINGS in $PerlIO::encoding::fallback
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 2.6k
Hi! I would continue in discussion started in last year in p5p mailing list: https://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239061.html Currently there is big mess with reporting warnings from Encode::encode() and Encode::decode() functions implemented by Encode module which is part of Perl core modules. Moreover mess is bigger, because Encode is used by PerlIO::encoding module which is internal Perl module. In perl you can enable some utf8 warnings by pragma warning. Next for Encode::encode() and Encode::decode() functions you can enable some warnings by Encode::ENCODE_WARN_ON_ERR check flag passed as optional argument to those functions. And third you can control some warnings via $PerlIO::encoding::fallback variable which is relevant for :encoding layer (which internally use Encode module and their encode/decode functions). This lead to couple of bug reports like Encode::encode() or decode() does not respect pragma warnings, or it does not respect check flag Encode::ENCODE_WARN_ON_ERR passed as argument. Some people hacked for some modules (e.g. UTF-16) to use only pragma warnings, which basically fully broke Encode::ENCODE_WARN_ON_ERR check flag. More complicated behaviour happened when using :encoding layer... https://rt.cpan.org/Public/Bug/Display.html?id=120505 https://rt.cpan.org/Public/Bug/Display.html?id=88592 https://github.com/dankogai/p5-encode/pull/26#issuecomment-235641347 https://rt.perl.org/Public/Bug/Display.html?id=128788 https://rt.cpan.org/Public/Bug/Display.html?id=116629 https://github.com/dankogai/p5-encode/issues/59 https://github.com/dankogai/p5-encode/commit/a6c2ba385875c2c03bd42350e23aef0188fb23b0 https://github.com/dankogai/p5-encode/commit/07c8adb58e55c7cf66b3d6673bf50010fe1a69ea As stated in previous discussion I'm proposing new behaviour: * Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which would tell Encode that it should report only those warnings which are currently enabled by pragma warnings. When Encode::ONLY_PRAGMA_WARNINGS is not set then Encode would report all warnings. The whole flag Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag Encode::ENCODE_WARN_ON_ERR is not set. * Add Encode::ONLY_PRAGMA_WARNINGS by default to :encoding layer variable $PerlIO::encoding::fallback, so by default every action on filehandle used by :encoding layer would report warnings according to pragma warnings. As this change affects both Perl & externally maintained Encode module on cpan, I'm opening ticket for Perl. In attachments are patches implementing above proposed behaviour. Encode patch is based on the last Encode version 2.91 (not in bleed yet).

Message body is not shown because sender requested not to inline it.

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org
On Sat, 01 Jul 2017 04:46:15 -0700, pali@cpan.org wrote: Show quoted text
> As stated in previous discussion I'm proposing new behaviour: > > * Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which > would tell Encode that it should report only those warnings which are > currently enabled by pragma warnings. When > Encode::ONLY_PRAGMA_WARNINGS > is not set then Encode would report all warnings. The whole flag > Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag > Encode::ENCODE_WARN_ON_ERR is not set. > > * Add Encode::ONLY_PRAGMA_WARNINGS by default to :encoding layer > variable $PerlIO::encoding::fallback, so by default every action on > filehandle used by :encoding layer would report warnings according to > pragma warnings. > > As this change affects both Perl & externally maintained Encode module > on cpan, I'm opening ticket for Perl. > > In attachments are patches implementing above proposed behaviour. > Encode > patch is based on the last Encode version 2.91 (not in bleed yet).
It think this is a great improvement. Thank you for all your work. -- Father Chrysostomos
Subject: Re: [perl #131683] Encode::ONLY_PRAGMA_WARNINGS in $PerlIO::encoding::fallback
To: perlbug-followup [...] perl.org
Date: Fri, 14 Jul 2017 09:47:08 +0200
From: pali [...] cpan.org
Download (untitled) / with headers
text/plain 1.2k
On Saturday 01 July 2017 06:08:08 Father Chrysostomos via RT wrote: Show quoted text
> On Sat, 01 Jul 2017 04:46:15 -0700, pali@cpan.org wrote:
> > As stated in previous discussion I'm proposing new behaviour: > > > > * Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which > > would tell Encode that it should report only those warnings which are > > currently enabled by pragma warnings. When > > Encode::ONLY_PRAGMA_WARNINGS > > is not set then Encode would report all warnings. The whole flag > > Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag > > Encode::ENCODE_WARN_ON_ERR is not set. > > > > * Add Encode::ONLY_PRAGMA_WARNINGS by default to :encoding layer > > variable $PerlIO::encoding::fallback, so by default every action on > > filehandle used by :encoding layer would report warnings according to > > pragma warnings. > > > > As this change affects both Perl & externally maintained Encode module > > on cpan, I'm opening ticket for Perl. > > > > In attachments are patches implementing above proposed behaviour. > > Encode > > patch is based on the last Encode version 2.91 (not in bleed yet).
> > It think this is a great improvement. Thank you for all your work.
Hi! Any comments for this new proposed behavior or implementation in patches? I would like to hear some feedback...
Date: Wed, 9 Aug 2017 14:02:58 +0200
To: perlbug-followup [...] perl.org
Subject: Re: [perl #131683] Encode::ONLY_PRAGMA_WARNINGS in $PerlIO::encoding::fallback
From: pali [...] cpan.org
Download (untitled) / with headers
text/plain 1.3k
On Saturday 01 July 2017 06:08:08 Father Chrysostomos via RT wrote: Show quoted text
> On Sat, 01 Jul 2017 04:46:15 -0700, pali@cpan.org wrote:
> > As stated in previous discussion I'm proposing new behaviour: > > > > * Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which > > would tell Encode that it should report only those warnings which are > > currently enabled by pragma warnings. When > > Encode::ONLY_PRAGMA_WARNINGS > > is not set then Encode would report all warnings. The whole flag > > Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag > > Encode::ENCODE_WARN_ON_ERR is not set. > > > > * Add Encode::ONLY_PRAGMA_WARNINGS by default to :encoding layer > > variable $PerlIO::encoding::fallback, so by default every action on > > filehandle used by :encoding layer would report warnings according to > > pragma warnings. > > > > As this change affects both Perl & externally maintained Encode module > > on cpan, I'm opening ticket for Perl. > > > > In attachments are patches implementing above proposed behaviour. > > Encode > > patch is based on the last Encode version 2.91 (not in bleed yet).
> > It think this is a great improvement. Thank you for all your work.
@Father Chrysostomos: Now patches and there fore more then month. Any comments for either new behavior or its implementation? I would like to hear your opinion... or if it is OK without any problem could it be merged?
CC: "bugs-bitbucket [...] rt.perl.org" <bugs-bitbucket [...] rt.perl.org>
Date: Thu, 10 Aug 2017 16:57:55 +0200
To: Perl5 Porters <perl5-porters [...] perl.org>
From: Leon Timmermans <fawaka [...] gmail.com>
Subject: Re: [perl #131683] Encode::ONLY_PRAGMA_WARNINGS in $PerlIO::encoding::fallback
Download (untitled) / with headers
text/plain 3.1k
On Sat, Jul 1, 2017 at 1:46 PM, via RT <perlbug-followup@perl.org> wrote:
Show quoted text
Hi!

I would continue in discussion started in last year in p5p mailing list:
https://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239061.html

Currently there is big mess with reporting warnings from
Encode::encode() and Encode::decode() functions implemented by Encode
module which is part of Perl core modules.

Moreover mess is bigger, because Encode is used by PerlIO::encoding
module which is internal Perl module.

In perl you can enable some utf8 warnings by pragma warning. Next for
Encode::encode() and Encode::decode() functions you can enable some
warnings by Encode::ENCODE_WARN_ON_ERR check flag passed as optional
argument to those functions. And third you can control some warnings via
$PerlIO::encoding::fallback variable which is relevant for :encoding
layer (which internally use Encode module and their encode/decode
functions).

This lead to couple of bug reports like Encode::encode() or decode()
does not respect pragma warnings, or it does not respect check flag
Encode::ENCODE_WARN_ON_ERR passed as argument. Some people hacked for
some modules (e.g. UTF-16) to use only pragma warnings, which basically
fully broke Encode::ENCODE_WARN_ON_ERR check flag. More complicated
behaviour happened when using :encoding layer...

https://rt.cpan.org/Public/Bug/Display.html?id=120505
https://rt.cpan.org/Public/Bug/Display.html?id=88592
https://github.com/dankogai/p5-encode/pull/26#issuecomment-235641347
https://rt.perl.org/Public/Bug/Display.html?id=128788
https://rt.cpan.org/Public/Bug/Display.html?id=116629
https://github.com/dankogai/p5-encode/issues/59
https://github.com/dankogai/p5-encode/commit/a6c2ba385875c2c03bd42350e23aef0188fb23b0
https://github.com/dankogai/p5-encode/commit/07c8adb58e55c7cf66b3d6673bf50010fe1a69ea

As stated in previous discussion I'm proposing new behaviour:

* Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which
would tell Encode that it should report only those warnings which are
currently enabled by pragma warnings. When Encode::ONLY_PRAGMA_WARNINGS
is not set then Encode would report all warnings. The whole flag
Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag
Encode::ENCODE_WARN_ON_ERR is not set.

* Add Encode::ONLY_PRAGMA_WARNINGS by default to :encoding layer
variable $PerlIO::encoding::fallback, so by default every action on
filehandle used by :encoding layer would report warnings according to
pragma warnings.

As this change affects both Perl & externally maintained Encode module
on cpan, I'm opening ticket for Perl.

In attachments are patches implementing above proposed behaviour. Encode
patch is based on the last Encode version 2.91 (not in bleed yet).

IMHO $PerlIO::encoding::fallback is a liability, and instead of digging a deeper hole we should probably fix it first. In particular:

  local $PerlIO::encoding::fallback = FB_CROAK;
  open my $fh, '<:encoding(utf-8)', $filename

May or may not DWIM depending on whether PerlIO::encoding was already loaded or not. Relying even further on it having a non-zero value doesn't feel wise to me.

Leon
From: pali [...] cpan.org
Subject: Re: [perl #131683] Encode::ONLY_PRAGMA_WARNINGS in $PerlIO::encoding::fallback
To: perlbug-followup [...] perl.org
Date: Mon, 14 Aug 2017 10:19:58 +0200
Download (untitled) / with headers
text/plain 3.6k
On Thursday 10 August 2017 07:58:22 Leon Timmermans via RT wrote: Show quoted text
> On Sat, Jul 1, 2017 at 1:46 PM, via RT <perlbug-followup@perl.org> wrote: >
> > Hi! > > > > I would continue in discussion started in last year in p5p mailing list: > > https://www.nntp.perl.org/group/perl.perl5.porters/2016/08/msg239061.html > > > > Currently there is big mess with reporting warnings from > > Encode::encode() and Encode::decode() functions implemented by Encode > > module which is part of Perl core modules. > > > > Moreover mess is bigger, because Encode is used by PerlIO::encoding > > module which is internal Perl module. > > > > In perl you can enable some utf8 warnings by pragma warning. Next for > > Encode::encode() and Encode::decode() functions you can enable some > > warnings by Encode::ENCODE_WARN_ON_ERR check flag passed as optional > > argument to those functions. And third you can control some warnings via > > $PerlIO::encoding::fallback variable which is relevant for :encoding > > layer (which internally use Encode module and their encode/decode > > functions). > > > > This lead to couple of bug reports like Encode::encode() or decode() > > does not respect pragma warnings, or it does not respect check flag > > Encode::ENCODE_WARN_ON_ERR passed as argument. Some people hacked for > > some modules (e.g. UTF-16) to use only pragma warnings, which basically > > fully broke Encode::ENCODE_WARN_ON_ERR check flag. More complicated > > behaviour happened when using :encoding layer... > > > > https://rt.cpan.org/Public/Bug/Display.html?id=120505 > > https://rt.cpan.org/Public/Bug/Display.html?id=88592 > > https://github.com/dankogai/p5-encode/pull/26#issuecomment-235641347 > > https://rt.perl.org/Public/Bug/Display.html?id=128788 > > https://rt.cpan.org/Public/Bug/Display.html?id=116629 > > https://github.com/dankogai/p5-encode/issues/59 > > https://github.com/dankogai/p5-encode/commit/ > > a6c2ba385875c2c03bd42350e23aef0188fb23b0 > > https://github.com/dankogai/p5-encode/commit/ > > 07c8adb58e55c7cf66b3d6673bf50010fe1a69ea > > > > As stated in previous discussion I'm proposing new behaviour: > > > > * Introduce new Encode check flag Encode::ONLY_PRAGMA_WARNINGS which > > would tell Encode that it should report only those warnings which are > > currently enabled by pragma warnings. When Encode::ONLY_PRAGMA_WARNINGS > > is not set then Encode would report all warnings. The whole flag > > Encode::ONLY_PRAGMA_WARNINGS would have no effect when flag > > Encode::ENCODE_WARN_ON_ERR is not set. > > > > * Add Encode::ONLY_PRAGMA_WARNINGS by default to :encoding layer > > variable $PerlIO::encoding::fallback, so by default every action on > > filehandle used by :encoding layer would report warnings according to > > pragma warnings. > > > > As this change affects both Perl & externally maintained Encode module > > on cpan, I'm opening ticket for Perl. > > > > In attachments are patches implementing above proposed behaviour. Encode > > patch is based on the last Encode version 2.91 (not in bleed yet). > >
> > IMHO $PerlIO::encoding::fallback is a liability, and instead of digging a > deeper hole we should probably fix it first. In particular: > > local $PerlIO::encoding::fallback = FB_CROAK; > open my $fh, '<:encoding(utf-8)', $filename
IMO this is different problem, not related to my proposed patches, which are about how to handle warnings. Show quoted text
> May or may not DWIM depending on whether PerlIO::encoding was already > loaded or not. Relying even further on it having a non-zero value doesn't > feel wise to me.
I agree that above :encoding layer without STOP AT PARTIAL flag in fallback should be fixed, but I think all those can be done independently of this effort about warnings done in this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org