Skip to content

Commit

Permalink
Leverage the Module Autoload path and save doing it ourselves.
Browse files Browse the repository at this point in the history 
This should be faster too as we won't automatically import all functions
to the session. Powershell now knows where to look before declaring they
don't exist.
  • Loading branch information
@Jackbennett
Jackbennett committed Mar 18, 2015
1 parent 004761b commit e811c60
Showing 1 changed file with 4 additions and 12 deletions.
16 changes: 4 additions & 12 deletions vendor/profile.ps1
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,10 @@
# Global modules directory
$global:PsGetDestinationModulePath = $PSScriptRoot + "\..\vendor\psmodules"
# Add Cmder modules directory to the autoload path.
$CmderModulePath = Join-path $PSScriptRoot "psmodules/"

This comment has been minimized.

Sign in to view

Copy link
@eMGabriel

eMGabriel Oct 1, 2015

This line is raising error in Powershell v2.0:

Join-Path : Cannot bind argument to parameter 'Path' because it is null.
At D:\Temp\Dev\App\Cmder 1.2pre\vendor\profile.ps1:3 char:29
+ $CmderModulePath = Join-path <<<<  $PSScriptRoot "psmodules/"
    + CategoryInfo          : InvalidData: (:) [Join-Path], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.JoinPathCommand

The cause of this is that $PSScriptRoot gets filled only when running in a psm1 module in PowerShell v2.0.
It has been fixed since PowerShell v3.0, but not in v2.0, which is default version in Windows 7:
https://connect.microsoft.com/PowerShell/feedback/details/522951/psscriptroot-only-works-with-modules

For PowerShell 2.0 compatibility, could you add the following line to initialize $PSScriptRoot in PowerShell v2.0? :

If ( $PSVersionTable.PSVersion.Major -lt 3 ) { $PSScriptRoot = Split-Path $MyInvocation.MyCommand.Path -Parent }

This comment has been minimized.

Sign in to view

Copy link
@Jackbennett

Jackbennett Oct 1, 2015

Author Contributor

I think we found that Autoloading was only added in Version 3 so maybe it's fine to just add -ErrorAction SilentlyContinue and leave it alone.

I think for v2 compatibility we'd have to also keep the code this commit replaces but under your suggestions version check and v2 users will get longer loading times when they open a console.

This comment has been minimized.

Sign in to view

Copy link
@eMGabriel

eMGabriel Oct 7, 2015

I have tried out $CmderModulePath = Join-path $PSScriptRoot "psmodules/" -ErrorAction SilentlyContinue but the error is still visible.
The cause: Get-Help about_CommonParameters mentions that ErrorAction parameter is ignored on terminating errors like when command parameter is empty or null :

The ErrorAction parameter has no effect on terminating errors (such as
missing data, parameters that are not valid, or insufficient
permissions) that prevent a command from completing successfully.

Could you give me an example when longer loading times will happen? What setup I should do? Because I did not find any loading speed change using my suggested modification. I have a bare PowerShell v2.0 installed without any external modules.
I would like to leave the decision to you, because I do not use PS on a daily basis, I only found this issue because PowerShell was the default shell start when running Cmder v1.2pre. So I reported it.

# Push to modules location
Push-Location -Path ($PsGetDestinationModulePath)

# Load modules from current directory
Import-Module .\PsGet\PsGet
Get-ChildItem -Exclude "PsGet" -Directory -Name | Foreach-Object {
Import-Module .\$_\$_
if( -not $env:PSModulePath.Contains($CmderModulePath) ){
$env:PSModulePath = $env:PSModulePath.Insert(0, "$CmderModulePath;")
}

# Come back to PWD
Pop-Location

# Set up a Cmder prompt, adding the git prompt parts inside git repos
function global:prompt {
$realLASTEXITCODE = $LASTEXITCODE
Expand Down

6 comments on commit e811c60

@LeoColomb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very satisfied with this change: it's not fully compatible with the portable behavior of Cmder.

  • It changes variable $env:PSModulePath globally on the computer
  • Without global:PsGetDestinationModulePath, PsGet don't install new module in the Cmder directory

I'm sure modules loading is improvable, but... 😃
What do you think?

@MartiUK
Copy link
Member

@MartiUK MartiUK commented on e811c60 May 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeoColomb What do you suggest?

@Jackbennett
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$env is a process level variable whereas Env: would be your user or system level scope I forget how to tell.

do $env:psModilePath in cmder then run a new powershell -noprofile and again $env:psModulePath you will see it's not there.

I would have to test how PsGetDestinationModulePath is working I haven't used oneGet.

@Jackbennett
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. [Environment]::GetEnvironmentVariable("psmodulepath", "process")
  2. [Environment]::GetEnvironmentVariable("psmodulepath", "user")
  3. [Environment]::GetEnvironmentVariable("psmodulepath", "machine")

@Jackbennett
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've spend 5 minutes searching, not to but rude just to clarify I may be ill informed.

I found psget/psget#105 and I suspect psget should fix this upstream. Using the most specific psModulePath location makes the most sense to me.

@LeoColomb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for clarification!

Please sign in to comment.