Make Data::Alias optional
Closed this issue · 9 comments
Data::Alias has continued to be a pain, causing bugs in other modules and holding back the ability to install. I would like to make it an optional requirement.
I don't think the aliasing feature is used all that often. As of 5.14 array and hash references can be used with things like push
and keys
eliminating a lot of its utility.
I see four options
- Leave things as they are
- Make Data::Alias a "recommended" module
- Make Data::Alias an "auto feature"
- Write Method::Signatures::Alias and make that an optional feature
I like 4 because it shields the user from changes in our aliasing implementation. LWP went into a similar situation between 5 and 6 when the requirements for their HTTPS support changed and suddenly everyone's HTTPS support broke. They created LWP::Protocol::https for people to depend on which pulls in the correct modules.
Method::Signatures::Alias would, right now, do little more than re-export Data::Alias::alias().
Personally I think:
- option 2 slows down installs which can be a pain if MS becomes a transient dependency from other modules
- option 3 is not ideal as it can be hard to understand why behaviour of MS may change between different installations
- so option 4 makes MS the most self sufficient and looks like a simple and robust solution
As of 5.14 array and hash references can be used with things like push and keys eliminating a lot of its utility.
Well, sounds like you're talking about the \$foo
feature. I've personally never found much use for that. OTOH, the is alias
trait is quite useful. For instance:
method extract_title ($text is alias)
{
if ($text =~ s/^Title:\s*(.*)$/m)
{
return $1;
}
return undef;
}
Would never miss the \$foo
thing. If the is alias
were to get harder for people to do, though, I would consider that pretty tragic.
That's my only input, really.
Oh yeah, I totally forgot about is alias
. Would it be acceptable if the behaviors were thus...
- If your code uses aliasing, all you have to do is declare a dependency on Method::Signatures::Alias
- Using aliasing automatically loads Method::Signatures::Alias
Which is to say, there would be no need to write use Method::Signatures::Alias
in your code. Only in the project dependencies.
@schwern wrote:
Data::Alias has continued to be a pain, causing bugs in other modules
and holding back the ability to install. I would like to make it an
optional requirement.
I don't have a problem with that.
I don't think the aliasing feature is used all that often.
I use it all the time. Most nested data structure manipulations are vastly
cleaner if you have an alias to a reference to the original node, rather
than a copy of the reference.
I'm fine if you can only get aliasing behaviour if you have Data::Alias
(or Method::Signatures::Alias, if you prefer) installed. So long as the
muggles get a sensible error message when they don't.
Damian
Which is to say, there would be no need to write use Method::Signatures::Alias in your code. Only in the project dependencies.
Hmmm ... not ideal. But perhaps acceptable. There certainly do seem to be a lot of Data::Alias issues.
Alternatively, is there a better/more compatible module to handle the aliasing features we want?
I'm not sure concerning a solution, but i can give a user feedback. The aliasing feature is the reason why i use M::S over M::S::Simple. I use it for all lists and hashes. Even though push() and keys() work on references, i love having @ and % sigils everywhere, to make it obvious what kind of data i'm manipulating.
This being said, this sounds pretty smooth to me:
"Which is to say, there would be no need to write use Method::Signatures::Alias in your code. Only in the project dependencies."
Revisiting this issue, I'm not sure why we couldn't just require
Data::Alias as soon as we run across any signature that needs it. If it doesn't load, we spit out an appropriate error message. Then we move it from 'requires' to 'recommends' in the makefile and call it a day.
I'll see if I can get this plan working while I'm doing a few other patchy bits.
This is apparently blocked by #71. I'll have to fix that one first.
Okay, we already are only require
ing Data::Alias when we have an aliased signature (at least we are now that #71 is fixed). AFA error messages go, it's the standard error message you get from trying to load a module that ain't there, but that's the same thing we do if you try to make a ro
parameter without Const::Fast, so I'm going to call that sufficient as well.
So really all there is left to do is a) move Data::Alias into the recommended section, and b) call it a day. Which I shall do forthwith.
(Side note: This could also be done with Const::Fast, although it would require a bit of effort with the tests to keep them quiet. If anyone wanted to make an issue for that, we could probably do it. But I'm not going to fiddle with it now.)