d9705996 / perl-toolbox

VSCode Extension for perlcritic Linting
https://marketplace.visualstudio.com/items?itemName=d9705996.perl-toolbox
12 stars 11 forks source link

Defaut cwd for perl -c to workspace #29

Open szTheory opened 4 years ago

szTheory commented 4 years ago

When spawning the perl -c process, set the current working directory to the workspace. This properly resolves relative paths with the -I flag. The end result is that use lib 'lib'; will work out of the box without any configuration when you want to use MyLocalPackage that is located in your project's local lib. This should make the initial setup a lot easier, especially for people new to Perl to get them up and running with less friction.

I'm running it locally now and so far so good, but I'd like to test it for a bit longer before saying it's ready.

dseynhae commented 4 years ago

Does that mean that the FindBin module will work?

use FindBin;
use lib "$FindBin::Bin";
szTheory commented 4 years ago

@beltaurus I just tried that and it didn't seem to work. I'm not sure why it wouldn't though. Going by the documentation for FindBin and specifically this part

$Bin - path to bin directory from where script was invoked

it seems like it should. Maybe you can get it working by fooling around with the this.configuration.path option it reads from? Have you been able to find any workarounds for yourself. It could give a hint for a more fundamental fix within the extension. If you want to try the build from my branch, just clone the repo and run vsce package to build the .vsix extension file, uninstall your current Perl Toolbox extension, then drag the .vsix into your Extensions window. Alternatively, you can run the extension in Debug mode from with VSC.

Also, I RTFM and noticed the include path section instructions.

"perl-toolbox.syntax.includePaths": [
    "$workspaceRoot/lib",
    "$workspaceRoot/local/lib/perl5"
]

Once I added that to my workspace settings, the change in my PR was no longer necessary. I think having those configuration options as default would be a good move. Then everything would just work out of the box for new users, which could help a bit with Perl adoption by lowering the barrier to entry. Plus it would have saved me a bit of time.

dseynhae commented 4 years ago

@szTheory : the workaround is very straightforward: Since the application is in my main folder, I just state in my launch.json:

"perl-toolbox.syntax.includePaths": [
        "${workspaceFolder}",
    ],

However, that is the wrong thing to do: this adds that folder to the @INC path for ALL Perl files and modules.... whereas it ONLY should apply to the files/modules asking for it (through the `use lib "$FindBin::Bin;" statement)...

I am surprised that the compile step -c flags it as a problem (especially if we compile it in the directory where the .pl file in question lives)...

If I run the Perl compilation in a terminal, where the application is actually located, I get:

$ perl -c <application>.pl
<application>.pl syntax OK

So I'm not sure why the compile step in the extension actually generates that error? I suspect that the extension copies ONLY <application>.pl to a temp directory, but that will indeed generate the compilation errors! The local *.pm files have to be copied to the same temp directory, in order for them to be found!!! (That is exactly the purpose of the $FindBin::Bin variable, to overcome that Perl no longer looks includes local directories in the @INC path (I think that started in V5.10)...

So when I saw the initial comment:

When spawning the perl -c process, set the current working directory to the workspace. I was very excited, because that should drop the problems related to finding libraries through $FindBin::Bin...

dseynhae commented 4 years ago

Please check #24 That shows that the Perl files do get copied somewhere else (and even get a different file name, which creates unnecessary errors flagged by PerlCritic).

So again, if Perl-Toolbox doesn't copy ALL files, with the correct name, to a temporary location, the `use lib "$FindBin::Bin" will not work, and flag a bunch of errors...