Skip to content

Commit

Permalink
IO::Path!new-from-absolute-path is a private method
Browse files Browse the repository at this point in the history
- so it doesn't need any named parameters, it can use positionals
- it doesn't need any defaults or coercing
  • Loading branch information
lizmat committed Feb 21, 2020
1 parent 3f637af commit 1d946e1
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/core.c/IO/Path.pm6
Expand Up @@ -21,7 +21,7 @@ my class IO::Path is Cool does IO {
);
}

method !new-from-absolute-path($path, :$SPEC = $*SPEC, Str() :$CWD = $*CWD) {
method !new-from-absolute-path($path, $SPEC, $CWD) {
self.bless(:$path, :$SPEC, :$CWD)!set-absolute($path);
}

Expand Down Expand Up @@ -331,7 +331,7 @@ my class IO::Path is Cool does IO {
}
}
$resolved = $volume ~ $sep if $resolved eq $volume;
IO::Path!new-from-absolute-path($resolved,:$!SPEC,:CWD($volume ~ $sep));
IO::Path!new-from-absolute-path($resolved,$!SPEC,$volume ~ $sep);
}
proto method parent(|) {*}
multi method parent(IO::Path:D: UInt:D $depth) {
Expand Down Expand Up @@ -412,7 +412,7 @@ my class IO::Path is Cool does IO {
@dirs.push('') if !@dirs; # need at least the rootdir
$path = join($!SPEC.dir-sep, $volume, @dirs);
}
my $dir = IO::Path!new-from-absolute-path($path,:$!SPEC,:CWD(self));
my $dir = IO::Path!new-from-absolute-path($path,$!SPEC,self.Str);

This comment has been minimized.

Copy link
@ugexe

ugexe Feb 21, 2020

Member

Is this actually equivilent? Previous self would have passed along a :CWD, which makes sense since "./foo".IO.Str gives "./foo", not the absolute path (which the CWD is needed for).

This comment has been minimized.

Copy link
@lizmat

lizmat Feb 21, 2020

Author Contributor

$!CWD is a Str, so I don't see the issue?

This comment has been minimized.

Copy link
@ugexe

ugexe Feb 21, 2020

Member

Then I suspect the original implementation was flawed as well. Str() should really have been coercing to an absolute value, and thus this should be calling self.absolute instead of self.Str

This comment has been minimized.

Copy link
@ugexe

ugexe Feb 21, 2020

Member
$ perl6 -e 'my $p1 = IO::Path.new("./xxx", :CWD("./zzz")); say IO::Path.new("./foo", :CWD($p1.absolute)).absolute'
/Users/ugexe/repos/test/zzz/xxx/foo

$ perl6 -e 'my $p1 = IO::Path.new("./xxx", :CWD("./zzz")); say IO::Path.new("./foo", :CWD($p1)).absolute'
/Users/ugexe/repos/test/xxx/foo

In other words: I would expect the two examples above to produce the same path

This comment has been minimized.

Copy link
@ugexe

ugexe Feb 21, 2020

Member

The above may not really apply to a 'new-from-absolute-path' perspective though


nqp::stmts(
nqp::unless(
Expand Down Expand Up @@ -559,7 +559,7 @@ my class IO::Path is Cool does IO {
$test.ACCEPTS($elem) && (
$absolute
?? take IO::Path!new-from-absolute-path(
$abspath ~ $elem,:$!SPEC,:$!CWD)
$abspath ~ $elem,$!SPEC,$!CWD)
!! take IO::Path.new($path ~ $elem,:$!SPEC,:$!CWD)
);
}
Expand All @@ -572,7 +572,7 @@ my class IO::Path is Cool does IO {
nqp::if(
$absolute,
(take IO::Path!new-from-absolute-path(
nqp::concat($abspath,$str-elem),:$!SPEC,:$!CWD)),
nqp::concat($abspath,$str-elem),$!SPEC,$!CWD)),
(take IO::Path.new(
nqp::concat($path,$str-elem),:$!SPEC,:$!CWD)),)));
nqp::closedir($dirh);
Expand Down

0 comments on commit 1d946e1

Please sign in to comment.