Add OO interface #41
Add OO interface #41
Conversation
zmughal
force-pushed
the
oo-interface
branch
3 times, most recently
from
e82deae
to
77aa627
Compare
The name indicates that it will remain the same despite changes to
`$ENV{PATH}`.
zmughal
force-pushed
the
oo-interface
branch
2 times, most recently
from
a9b7707
to
f534a13
Compare
| sub new { | ||
| my ($class, %opts) = @_; | ||
|
|
||
| my $osname = exists $opts{os} ? $opts{os} : $^O; |
There was a problem hiding this comment.
I'm not sure $^O is quite right for this, it has a lot of detail for unix like operating systems and not enough for windows. I think we want to know if it is unix, or win/vms/cyg/os9, and then we want something like win+powershell win+bash.
There was a problem hiding this comment.
Maybe we can have opts for os and shell. I still don't think $^O is right for os as Windows9x and WindowsNT are both MSWin32 unfortunately. I'm not too enthused about inventing our own categories for os, but we may have to. Note #41 (review) and #42
| $self->{PATHEXT} = $self->_default_pathext; | ||
|
|
||
| if( exists $opts{fixed_paths} ) { | ||
| $self->{fixed_paths} = $opts{fixed_paths}; |
There was a problem hiding this comment.
I would like to delete from %opts and error if there is anything left over, that way we can add new options in the future, and if someone is trying to use those options on an older File::Which they will get an error.
| if ( $self->_is_win ) { | ||
| # WinNT. PATHEXT might be set on Cygwin, but not used. | ||
| if ( $ENV{PATHEXT} ) { | ||
| push @PATHEXT, split /;/, $ENV{PATHEXT}; |
There was a problem hiding this comment.
I know this is a Windows environment variable, but I think we should use Env qw( @PATHEXT ) for this and other list type environment variables. This will make it easier to test windows behavior on non-windows OS. Any tests that test default behavior can also use Env.
There was a problem hiding this comment.
Env is core as of Perl 5, so introducing it as a prereq shouldn't be a problem.
| if ( $ENV{PATHEXT} ) { | ||
| push @PATHEXT, split /;/, $ENV{PATHEXT}; | ||
| } else { | ||
| # Win9X or other: doesn't have PATHEXT, so needs hardcoded. |
There was a problem hiding this comment.
I guess this isn't related to this PR, but if PATHEXT isn't supported on Windows 9x or DOS, then we shouldn't use it on those platforms.
There was a problem hiding this comment.
but we should at least consider that we may need to treat DOS/Windows9x as a different OS than WindowsNT.
| $self->_is_win || $self->_is_vms || $self->_is_mac; | ||
| } | ||
| our $IMPLICIT_CURRENT_DIR = do { | ||
| File::Which->new->_default_implicit_current_dir; |
There was a problem hiding this comment.
I apologize for introducing this interface now. But we have to support it :(
There was a problem hiding this comment.
Just a comment (you are already doing that).
|
I think this is the right approach.
|
|
When you are writing the documentation, you can assume that we will bump the version to 2.00 when we add the OO interface. OO examples should look like this: use File::Which 2.00;
my $which = File::Which->new; |
DRAFT: Start of OO interface.
This PR is just a placeholder. It still needs improvements to the tests, adding
the
os_shelloption (tentative name), documentation, and setting thePATH/PATHEXTas options.Connects to #40.