Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix $/ being interpolated into a regular expression #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Corion
Copy link

@Corion Corion commented Oct 20, 2018

File::Slurp doesn't guard against $/ being set to something that looks like a regular expression. This fixes that by quoting the contents of $/ unless paragraph mode is on.

This doesn't guard against

$/ = \1024;

but that's not really relevant I presume.

@genio
Copy link
Collaborator

genio commented Oct 20, 2018

@Corion Please accept my apologies in advance. Right now, we're focused solely on getting 5.30 compliance with the binmode + sysread/write/open problem. Once that is sorted, we'll happily come back around to enhancements.

I'll leave this open here, but it likely won't be reviewed/merged until after the aforementioned fixes happen.

@Corion
Copy link
Author

Corion commented Oct 21, 2018

Thank you very much!

I don't mind if the change gets delayed - I'm waiting since 2012 for fixes on Windows ( See https://rt.cpan.org/Public/Bug/Display.html?id=84918 ), so I'm really happy that anything is moving at all with File::Slurp.

My change here isn't necessary to keep working programs from breaking.

@Corion
Copy link
Author

Corion commented Nov 23, 2018

I've fixed the merge conflict to track development and restructuring.

@genio
Copy link
Collaborator

genio commented Feb 13, 2019

Please rebase this when you get a chance and we'll start reviewing. Thanks!

@Corion
Copy link
Author

Corion commented Feb 13, 2019

I've rebased it just now

Thank you for looking after the module!

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.

2 participants