bscan / PerlNavigator

Perl Language Server that includes syntax checking, perl critic, and code navigation
MIT License
198 stars 39 forks source link

Two commits (can be cherrypicked): Custom Env vars for Perl in settings, and convert tilda to home in paths #119

Closed nugged closed 6 months ago

nugged commented 6 months ago

Feature 1:

Add support for custom Perl environment variables

Introduced two new environment variables: perlEnv and perlEnvAdd.

Feature 2:

Add support for expanding leading tildas in paths

nugged commented 6 months ago

NOTE: I tested this under MacOS (no Windows and I suspect it might be a different way of substituting Windows "home" or other relativeness).

bscan commented 6 months ago

This looks great, thanks for putting this together! I'll do some testing this weekend and merge it in. Some comments:

I don't think we'd need to disable the tilde expansion. Currently, tilde's simply don't work so having it on all the time is probably the best. Looks like they considered adding it in node, but decided against it: https://github.com/nodejs/node/issues/684 And people seem to recommend untildify, which has 14 million weekly downloads and does the same thing ( os.homedir ) https://github.com/sindresorhus/untildify/blob/a901e25ef782d93df1eba04ed48f56c79d157c74/index.js#L10 Although I greatly prefer your approach over bringing in a dependency for one line of code.

On Windows, the equivalent is probably %USERPROFILE% although far less used. We can probably just ignore it for now. I can test on Windows and Linux just to make sure nothing breaks, but I think it would be fine.

PerlEnv and PerlEnvAdd both make sense as well. I suspect Add is the more common case, where someone just needs to add or override a variable. Clearing out variables might have some unexpected effects, especially if someone's perl setup uses PERL5LIB or a PERLCRITIC profile. We can use the README to point people toward preferring PerlEnvAdd if needed.

Thanks!

nugged commented 6 months ago

PerlEnv and PerlEnvAdd both make sense as well. I suspect Add is the more common case, where someone just needs to add or override a variable. Clearing out variables might have some unexpected effects, especially if someone's perl setup uses PERL5LIB or a PERLCRITIC profile. We can use the README to point people toward preferring PerlEnvAdd if needed.

Good point. I reworked the idea of PerlEnv: two variables:

More clean, and added by default, and perlEnvAdd can be used rarely.

nugged commented 6 months ago

Also: removed

Add expandLeadTildasInPaths Setting for Optional Tilda Expansion

commit at all, no need to on/off tildas' expansion.

nugged commented 6 months ago

... and removed Windows "placeholder" ~\\ which is actually not.

bscan commented 6 months ago

This is great, my apologies for the slow merge. Thanks again for putting this together!