Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reusing a PV is leading to common mistakes #16

Closed
atoomic opened this issue Apr 10, 2020 · 2 comments · Fixed by #17
Closed

Reusing a PV is leading to common mistakes #16

atoomic opened this issue Apr 10, 2020 · 2 comments · Fixed by #17

Comments

@atoomic
Copy link
Contributor

atoomic commented Apr 10, 2020

This is probably not a recommended usage, but this seems what many users will do when start using the module: recycling PVs to use guard....
this could lead to unexpected behaviors... we should probably either try to warn or fix the issue

#!perl

use strict;
use warnings;

use File::Path     qw(make_path);  
use File::pushd;

use Cwd;

use Test::More;

my $root = getcwd;

make_path( $root . "/a/b/c" );
ok -d $root . "/a/b/c";

my $in_dir = pushd( "a" );
is getcwd, "$root/a", "we are in 'a'";

# undef($in_dir) or use another PV 'my $in_dir2'
$in_dir = pushd( "$root/a/b" );
is getcwd, "$root/a/b", "we are in 'a/b'";

done_testing;

output:

╰─> perl test.pl
ok 1
ok 2 - we are in 'a'
not ok 3 - we are in 'a/b'
#   Failed test 'we are in 'a/b''
#   at test.pl line 23.
#          got: '/Users/xxxx/workspace'
#     expected: '/Users/xxxx/workspace/a/b'
1..3
# Looks like you failed 1 test of 3.

We can fix by either:

  • forcing undef on the SV: undef($in_dir)
  • using another scope
  • using another PV...
@xdg
Copy link
Contributor

xdg commented Apr 13, 2020

Seems like a doc-fix is what's needed. If users don't understand object destruction semantics, there's not much I think we should be doing to avoid letting them shoot themselves in the foot.

@atoomic
Copy link
Contributor Author

atoomic commented Apr 15, 2020

doc-fix seems fair IMO, we should probably mention it in a Common Mistakes or Restrictions section, I can work something like this.

Another issue would be to use two objects in the same scope :-)

atoomic added a commit to atoomic/File-pushd that referenced this issue Apr 15, 2020
atoomic added a commit to atoomic/File-pushd that referenced this issue Jun 5, 2020
@xdg xdg closed this as completed in #17 Jun 5, 2020
xdg pushed a commit that referenced this issue Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants