thaljef / Pinto

Curate your own repository of Perl modules
https://metacpan.org/module/Pinto::Manual
66 stars 49 forks source link

implement a 'look' command like cpanm #184

Closed tartansandal closed 9 years ago

thaljef commented 9 years ago

This looks really good, but we can't release it until it works reasonably with a remote repository. There are two choices for that:

  1. Implement support for remote repositories by creating a subclass of Pinto::Remote::Action that fetches the remote tarball first (see Pinto::Remote::Action::Installl for example).
  2. Make App::Pinto::Command::look::execute() bail out with an appropriate error message if the repository is remote (i.e. starts with /https?/). Then we'll add support for remote repositories later.

Either is fine with me.

tartansandal commented 9 years ago

Okay. Implemented 3 patches to fix copy-n-paste errors, 1 to make SHELL a File , and 1 to die if the remote repository is remote (inspired by the 'init' command). Opening distros from remote repositories makes good sense, but I'm a little short on time ATM.

I'm using next::method() from mro 'cause its been so long and I can't remember how to use SUPER anymore. Really wanted to use a 'before' modifier, but App::Cmd does not seem to role that way. Added MRO::compat so users of RHEL5 will be happier. Opinions most welcome ;-)

thaljef commented 9 years ago

App::Cmd isn't based on Moo/Moose, so method modifiers are not an option there. I don't have anything against MRO (DBIx::Class is already using it). But for the sake of consistency, we have to choose one way or the other. Currently we have no next::method() calls in the code but two of these:

App/Pinto/Command/help.pm:24:    my $rv = $self->SUPER::execute( $opts, $args );
App/Pinto/Command/nop.pm:29:    $self->SUPER::validate_args( $opts, $args );
tartansandal commented 9 years ago

I reckon we stick with the status quo -- now that I've been reminded of how to use SUPER once again :-)

We don't have complex/multiple inheritance under App::Cmd so the whole MRO thing is not important.

Will get onto a patch for this an hour or so.

thaljef commented 9 years ago

Will get onto a patch for this an hour or so.

Already done: bb113a9000

thaljef commented 9 years ago

I already merged this, but I just realized this command needs at stack option too. Whenever a command accepts package names like Foo::Bar as a target, then it needs to know which stack to resolve that against, because every stack could have a different version of Foo::Bar. This is an important feature of Pinto's behavior. I'll try to fix this so you can see what I mean.

thaljef commented 9 years ago

I rewrote some of this in 09c626bd. Mostly I just re-arranged things. But I did make one interface change by removing the --shell command line option in favor of a PINTO_SHELL environment variable, which is consistent with other parts of the interface, such as PINTO_EDITOR and PINTO_PAGER. Thanks to you, this is a very nice addition to Pinto :smiley_cat:

tartansandal commented 9 years ago

Nice reorg. "Look"ing good ;-)

Kahlil (Kal) Hodgson GPG: C9A02289 Head of Technology (m) +61 (0) 4 2573 0382 DealMax Pty Ltd

Suite 1416 401 Docklands Drive Docklands VIC 3008 Australia

"All parts should go together without forcing. You must remember that the parts you are reassembling were disassembled by you. Therefore, if you can't get them together again, there must be a reason. By all means, do not use a hammer." -- IBM maintenance manual, 1925

On 4 March 2015 at 05:14, Jeffrey Ryan Thalhammer notifications@github.com wrote:

I rewrote some of this in 09c626b https://github.com/thaljef/Pinto/commit/09c626bd6c2b3bbec27b8f103d214a486a201ffe. Mostly I just re-arranged things. But I did make one interface change by removing the --shell command line option in favor of a PINTO_SHELL environment variable, which is consistent with other parts of the interface, such as PINTO_EDITOR and PINTO_PAGER. Thanks to you, this is a very nice addition to Pinto [image: :smiley_cat:]

— Reply to this email directly or view it on GitHub https://github.com/thaljef/Pinto/pull/184#issuecomment-77001599.