Skip to content

Conversation

@mwarin
Copy link
Contributor

@mwarin mwarin commented Nov 7, 2024

Fix for issue #156, triggered by https://hathitrust.atlassian.net/issues/GS-11436.

@mwarin mwarin requested a review from aelkiss November 7, 2024 22:32
Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love having to do it, but I think the fix is fine, and it's good to be generous in what we accept. It might be good to ensure that the use open ... is lexically scoped to this block and won't inadvertently break stuff elsewhere?

@mwarin
Copy link
Contributor Author

mwarin commented Nov 8, 2024

I think this proves the lexicalness of its scope:

use strict;                              
use warnings;                            
                                         
no_pragma();                             
use_pragma();                            
no_pragma();                             
                                         
# Should print the unicode as expected   
sub use_pragma {                         
    use open ':std', ':encoding(UTF-8)'; 
    print "Using pragma\n";              
    open(F, "utf_chars.txt") or die;     
    while (<F>) {                        
        print;                           
    }                                    
    close(F);                            
    print "-----\n";                     
}                                        
                                         
# Should print the unicode as garbled goo
sub no_pragma {                          
    print "NOT using pragma\n";          
    open(F, "utf_chars.txt") or die;     
    while (<F>) {                        
        print;                           
    }                                    
    close(F);                            
    print "-----\n";                     
}                                        

Just make a utf_chars.txt and put some s in it or something, and watch it print the same stuff differently depending on the scope.

@mwarin mwarin merged commit d7e4839 into main Nov 11, 2024
@mwarin mwarin deleted the issue-156-checksum-bom branch November 11, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants