search

Abigail / geography--states

Map states and provinces to their codes, and vica versa
4 stars 1 forks source link

__DATA__ removal commit #1

Open aggrolite opened 10 years ago

aggrolite commented 10 years ago

Hi Abigail,

Was the change removing __DATA__ (commit 8865ebeb882cda5e2620b4478aa0f982b6496016) ever released to CPAN?

The reason I ask is because the commit message describes that this change avoids character issues, which may be related to an issue I had recently.

Thanks!

skington commented 9 years ago

Seconded: under perl 5.14.2, loading Geography::States produces all sorts of "utf8 "\xE1" does not map to Unicode at /opt/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/Geography/States.pm line 50, line 68" errors.

aggrolite commented 9 years ago

@skington IIRC I saw the same running 5.8.9

skington commented 9 years ago

I narrowed it down to a use open pragma, so I've just released Geography::States::NoUnicodeWarnings to the CPAN.

karenetheridge commented 9 years ago

@skington are you experiencing issues with Geography::States even when you drop all use open pragmas? You might be saying to perl "treat all handles as utf8", but that's wrong when GS deals directly with latin1 data (which is ok, because none of the strings it deals with extend outside the first 256 characters in unicode). Pragmas which affect the entire runtime state are best avoided.

skington commented 9 years ago

I'm pretty sure it's a use open pragma, yes, because (a) that was the thing that was in our "enable all of these pragmata in all of our code" module at work that I narrowed the bug down to, and (b) the fix appears to be to say

{
    use open ':std';
    Module::Load::load('Geography::States');
}

which is all that Geography::States::NoUnicodeWarnings does.

I fully appreciate your point that you shouldn't apply pragmata universally - indeed, that's why I wrote warnings::everywhere. But given that our codebase - via the internal class our::way - decided that we should force UTF8 on all opens to avoid Unicode errors, and Geography::States in its released version produces warnings in those conditions, the alternatives are either to make sure Geography::States is always loaded before our::way (which might not be possible in all circumstances), or make sure that Geography::States is explicitly loaded with the UTF8-checking turned off.

So the latter seemed the best way of patching around things.

Of course, once Abigail, or someone else, releases the version of Geography::States in GitHub to the CPAN, there won't be any need for such a workaround, and I'll happily retire it from the CPAN.