search

doy / package-stash-xs

faster and more correct implementation of the Package::Stash API
4 stars 3 forks source link

Make module_name_rx extendable #6

Open rurban opened 9 years ago

rurban commented 9 years ago

One might not accept the parser-based limitations of valid module names, e.g. by creating packages in XS. E.g I use the int? type as type alias for (int | undef), other might want to use unicode names.

This way you only need to asjust one variable: $Module::Runtime::module_name_rx

karenetheridge commented 9 years ago

Shouldn't the alteration of $Module::Runtime::module_name_rx be localized to the scope in which it's needed? otherwise, "interesting" things could happen elsewhere in the program.

I do support the desire to change the regex that is used, but I'm not sure if this is the way to do it.

rurban commented 9 years ago

Yes, you could localize your change to this package variable. This module however just optionally picks such a change to the variable in the other package. On Jun 9, 2015 10:53, "Karen Etheridge" notifications@github.com wrote:

Shouldn't the alteration of $Module::Runtime::module_name_rx be localized to the scope in which it's needed? otherwise, "interesting" things could happen elsewhere in the program.

I do support the desire to change the regex that is used, but I'm not sure if this is the way to do it.

— Reply to this email directly or view it on GitHub https://github.com/doy/package-stash-xs/pull/6#issuecomment-110430304.

ribasushi commented 8 years ago

Given this patch has a very niche application and tangibly complicates the code (adding a hardcoded copy of a part of Module::Runtime, with an implicit logic fork depending if M::R has been loaded), I recommend holding off until https://github.com/doy/package-stash/issues/15 is resolved, at which point the only relevant part of this PR would be the (expected to pass) addition to t/extension.t

karenetheridge commented 5 years ago

This distribution now lives at https://github.com/moose/Package-Stash-XS -- let's finish this up there.