Implement Add-PoshGitToProfile command #361

rkeithhill · 2017-01-08T00:38:45Z

Offer to update their current/all host profile if posh-git not detected in their profile scripts. Note: we do not offer to update the "all users" profile scripts because that requires elevation and probably not worth it.

Add Pester tests for the new Utils.ps1 functions.

Offer to update their current/all host profile if posh-git not detected in their profile scripts. Note: we do not offer to update the "all users" profile scripts because that requires elevation and probably not worth it. Add Pester tests for the new Utils.ps1 functions.

dahlbyk

This is looking great!

dahlbyk · 2017-01-08T04:28:32Z

Utils.ps1

+        # Doesn't exist, so create it
+        New-Item -Path $profilePath -ItemType File
+        $profileContent = @()
+        $encoding = 'ascii'


Let's go with utf8 here to make sure we can handle paths with non-ASCII characters.

dahlbyk · 2017-01-08T04:53:35Z

Utils.ps1

+
+    # Check if the location of this module file is in the PSModulePath
+    if (Test-InModulePath $ScriptRoot) {
+        $profileContent += "Import-Module posh-git"


Should we instead check Get-Module posh-git -ListAvailable to see if it finds this particular instance of the module? Or check that it will find any posh-git module and consider that good enough?

I'm almost tempted to just skip straight to always hard-coding the full module path.

I would avoid hard-coding the module path as a "bad practice" for PowerShell modules. I can switch to using Get-Module -ListAvailable.

Doh, that makes the Pester test a little harder. Oh well, hopefully not too much harder. :-)

OK this is making the Pester tests a lot harder. For some reason, Mock Get-Module is not working. Will look at it more tomorrow.

dahlbyk · 2017-01-08T05:57:09Z

install.ps1

@@ -1,5 +1,8 @@
 param([switch]$WhatIf = $false)

+# Dot source for Get-FileEncoding
+. $PSScriptRoot\Utils.ps1


$PSScriptRoot not supported in v2, right? Move $installDir here from below?

Thought I caught that. We can just use Split-Path $MyInvocation.MyCommand.Path -Parent instead.

dahlbyk · 2017-01-08T06:00:43Z

posh-git.psm1

@@ -92,6 +92,57 @@ if (!$currentPromptDef -or ($currentPromptDef -eq $defaultPromptDef)) {
    Set-Item Function:\prompt -Value $poshGitPromptScriptBlock
 }

+# If running interactive, check if user want's `import-module posh-git` added to their profile
+if (!$NoProfileCheck -and ($MyInvocation.ScriptName.Length -eq 0)) {


Is the Length check more idiomatic than !$MyInvocation.ScriptName?

Also, s/want's/wants above.

dahlbyk · 2017-01-08T06:13:08Z

test/Utils.Tests.ps1

+                @($content)[0] | Should BeExactly "Import-Module '$scriptRoot\posh-git.psd1'"
+            }
+            finally {
+                Remove-Item $profilePath -ErrorAction SilentlyContinue


BeforeEach { $profilePath = [System.IO.Path]::GetTempFileName() } AfterEach { Remove-Item $profilePath -ErrorAction SilentlyContinue }

❓

Heh, I started with that. Then wrote a few tests that didn't need $profilePath and removed Before/AfterEach. Then I moved those tests to a different Context. Fixed.

Remove $PSScriptRoot from install.ps1.

dahlbyk · 2017-01-08T20:03:45Z

install.ps1

@@ -1,5 +1,9 @@
 param([switch]$WhatIf = $false)

+# Dot source for Get-FileEncoding
+$scriptRoot = Split-Path $MyInvocation.MyCommand.Path -Parent


We have $installDir = Split-Path $MyInvocation.MyCommand.Path -Parent on L22. Rename this one and delete that line? (Or keep this name and fix uses of $installDir.)

dahlbyk · 2017-01-08T20:05:33Z

Minor nitpick, otherwise revisions look good. Just need to fix up how we detect if we should Import-Module posh-git!

However to get the Pester tests to work I had to put the Get-Module -List command in its own function and I mock that function. Mocking Get-Module was resulting in some really weird bug about a write error trying to set PSEdition.

dahlbyk · 2017-01-09T02:50:12Z

Is this ready for a final review? (Go ahead and remove WIP from title if so.)

rkeithhill · 2017-01-09T04:19:57Z

Yes. Ready for final review.

rkeithhill · 2017-01-09T04:42:18Z

Hmm, hold off on this a bit. Joel says ScriptName is empty in implicit loading scenarios and he's right. Let's see if we can use Get-PSCallstack instead.

Committing this so I can test it on some other systems - via remoting.

rkeithhill · 2017-01-10T04:01:05Z

I'm becoming quite nervous about this PR. Using Get-PSCallstack has its issues as well. In general I thought a stack count of 2 would be reliably indicative of importing from the prompt e.g. here is a manual import call stack:

PS C:\Users\Keith> ipmo posh-git

Command          : posh-git.psm1
Location         : posh-git.psm1: line 95
Arguments        : {}
ScriptName       : C:\Users\Keith\Documents\WindowsPowerShell\Modules\posh-git\0.7.0\posh-git.psm1
ScriptLineNumber : 95
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : Get-PSCallstack | fl * | Out-String
FunctionName     : <ScriptBlock>

Command          : <ScriptBlock>
Location         : <No file>
Arguments        : {}
ScriptName       :
ScriptLineNumber : 1
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : ipmo posh-git
FunctionName     : <ScriptBlock>

And when the PowerShell console profile runs, the stack depth is three:

Command          : posh-git.psm1
Location         : posh-git.psm1: line 95
Arguments        : {}
ScriptName       : C:\Users\Keith\Documents\WindowsPowerShell\Modules\posh-git\0.7.0\posh-git.psm1
ScriptLineNumber : 95
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : Get-PSCallstack | fl * | Out-String
FunctionName     : <ScriptBlock>

Command          : profile.ps1
Location         : profile.ps1: line 9
Arguments        : {}
ScriptName       : C:\Users\Keith\Documents\WindowsPowerShell\profile.ps1
ScriptLineNumber : 9
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : Import-Module posh-git
FunctionName     : <ScriptBlock>

Command          : <ScriptBlock>
Location         : <No file>
Arguments        : {}
ScriptName       :
ScriptLineNumber : 1
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : . 'C:\Users\Keith\Documents\WindowsPowerShell\profile.ps1'
FunctionName     : <ScriptBlock>

But then I started to notice that the PowerShell in VSCode on my machine keep stalling during init. And of course it was stalling because of the query to modify the import. Sigh... There is something about the PowerShell extension custom host that results in a stack depth one less than expected when processing profiles e.g.:

Command          : posh-git.psm1
Location         : posh-git.psm1: line 96
Arguments        : {}
ScriptName       : C:\\Users\\Keith\\Documents\\WindowsPowerShell\\Modules\\posh-git\\0.7.0\\posh-git.psm1
ScriptLineNumber : 96
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : Get-PSCallstack | Fl * | Out-String
FunctionName     : <ScriptBlock>

Command          : profile.ps1
Location         : profile.ps1: line 10
Arguments        : {}
ScriptName       : C:\\Users\\Keith\\Documents\\WindowsPowerShell\\profile.ps1
ScriptLineNumber : 10
InvocationInfo   : System.Management.Automation.InvocationInfo
Position         : Get-GitStatus
FunctionName     : <ScriptBlock>

Another issue is that if posh-git hasn't been imported yet and you execute one of its commands, it will be auto-imported. That will cause the Modify Profile prompt to appear. That could be confusing.

How about we rejigger this work into a command like Add-PoshGitToProfile? It will result in a pretty simple install process e.g.:

Install-Module posh-git -Scope User
Add-PoshGitToProfile

Otherwise I'm worried there will be some unanticipated scenario (other custom hosts, remoting, etc) that will result in posh-git stalling the script waiting for input.

rkeithhill · 2017-01-10T19:27:13Z

Looking that the Docker instructions for getting going with the posh-docker PowerShell module see the section Set up tab completion in PowerShell. Their profile mod approach is simply:

Install-Module -Scope CurrentUser posh-docker -Force
Add-Content $PROFILE "`nImport-Module posh-docker"

I like the simplicity. They could have also mentioned that if you want posh-docker loaded in all hosts, then use:

Install-Module -Scope CurrentUser posh-docker -Force
Add-Content $PROFILE.CurrentUserAllHosts "`nImport-Module posh-docker"

And Add-Content seems to preserve the original file's encoding.

dahlbyk · 2017-01-11T04:19:39Z

If you're nervous, I'm nervous. Let's skip the magic. I appreciate the simplicity of Docker's approach, but wouldn't mind being a bit more helpful. How about:

Add-PoshGitToProfile [-AllHosts] [-Force]

-AllHosts would switch from CurrentUserCurrentHost to CurrentUserAllHosts.
-Force would skip our check if posh-git is already being imported by a profile.
We would keep our check to see if we can import by name or need to import the full path.
The Chocolatey install would replace its complexity with an import and Add-PoshGitToProfile

And Add-Content seems to preserve the original file's encoding.

And specifying -Encoding UTF8 seems to provide the encoding to use if the file doesn't exist. 💯

We'll still need to handle encoding (we don't today) in Chocolatey install/uninstall, but Add-Content seems sufficient for Add-PoshGitToProfile.

rkeithhill · 2017-01-11T04:44:58Z

I appreciate the simplicity of Docker's approach, but wouldn't mind being a bit more helpful.

Agreed. Everything else sounds good as well. I'm starting on it.

Now there is an Add-PoshGitToProfile command. I have a couple of Pester tests but I need to add a few more.

Not reliable on PS v2. Add tests.

rkeithhill · 2017-01-12T06:21:00Z

I'm ready to merge this. @dahlbyk Do you have time to review this?

…port

rkeithhill requested a review from dahlbyk January 8, 2017 01:11

Clean up comments and import choice script.

f3d4973

dahlbyk requested changes Jan 8, 2017

View reviewed changes

rkeithhill added 2 commits January 8, 2017 00:48

Change to utf8 and use Before/AfterEach.

2ac0b18

Remove $PSScriptRoot from install.ps1.

Simplfy check of $MyInvocation.ScriptName.

bbabf70

dahlbyk reviewed Jan 8, 2017

View reviewed changes

rkeithhill added 2 commits January 8, 2017 13:22

Remove extra call to Split-Path $MyInvocation ...

8eac200

Use Get-Module -List instead of PSModulePath.

ff4646a

However to get the Pester tests to work I had to put the Get-Module -List command in its own function and I mock that function. Mocking Get-Module was resulting in some really weird bug about a write error trying to set PSEdition.

dahlbyk mentioned this pull request Jan 9, 2017

Roadmap to 1.0 #328

Open

13 tasks

rkeithhill changed the title ~~WIP check user profile during interactive import.~~ Check user profile during interactive import. Jan 9, 2017

rkeithhill changed the title ~~Check user profile during interactive import.~~ WIP Check user profile during interactive import. Jan 9, 2017

See if callstack check works better.

972d539

Committing this so I can test it on some other systems - via remoting.

rkeithhill added 2 commits January 10, 2017 23:44

Remove modify profile from module import.

f51b3db

Now there is an Add-PoshGitToProfile command. I have a couple of Pester tests but I need to add a few more.

Do not use Get-Module -List posh-git.

539f714

Not reliable on PS v2. Add tests.

rkeithhill changed the title ~~WIP Check user profile during interactive import.~~ Implement Add-PoshGitToProfile command Jan 12, 2017

Merge branch 'master' into rkeithhill/modify-profile-on-interative-im…

48511d5

…port

dahlbyk merged commit a99b031 into master Jan 16, 2017

dahlbyk deleted the rkeithhill/modify-profile-on-interative-import branch January 16, 2017 05:47

dahlbyk mentioned this pull request Jan 16, 2017

Chocolatey Simplification + Uninstall #358

Merged

dahlbyk mentioned this pull request Feb 2, 2017

Can't install latest release #381

Closed

dahlbyk mentioned this pull request Feb 6, 2017

Release v0.7.0 #400

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Add-PoshGitToProfile command #361

Implement Add-PoshGitToProfile command #361

rkeithhill commented Jan 8, 2017

dahlbyk left a comment

dahlbyk Jan 8, 2017

rkeithhill Jan 8, 2017

dahlbyk Jan 8, 2017

rkeithhill Jan 8, 2017

rkeithhill Jan 8, 2017

rkeithhill Jan 8, 2017

dahlbyk Jan 8, 2017

rkeithhill Jan 8, 2017

dahlbyk Jan 8, 2017

dahlbyk Jan 8, 2017

rkeithhill Jan 8, 2017

dahlbyk Jan 8, 2017 •

edited

Loading

dahlbyk commented Jan 8, 2017

dahlbyk commented Jan 9, 2017

rkeithhill commented Jan 9, 2017

rkeithhill commented Jan 9, 2017

rkeithhill commented Jan 10, 2017

rkeithhill commented Jan 10, 2017

dahlbyk commented Jan 11, 2017

rkeithhill commented Jan 11, 2017

rkeithhill commented Jan 12, 2017 •

edited

Loading

Implement Add-PoshGitToProfile command #361

Implement Add-PoshGitToProfile command #361

Conversation

rkeithhill commented Jan 8, 2017

dahlbyk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dahlbyk Jan 8, 2017 • edited Loading

Choose a reason for hiding this comment

dahlbyk commented Jan 8, 2017

dahlbyk commented Jan 9, 2017

rkeithhill commented Jan 9, 2017

rkeithhill commented Jan 9, 2017

rkeithhill commented Jan 10, 2017

rkeithhill commented Jan 10, 2017

dahlbyk commented Jan 11, 2017

rkeithhill commented Jan 11, 2017

rkeithhill commented Jan 12, 2017 • edited Loading

dahlbyk Jan 8, 2017 •

edited

Loading

rkeithhill commented Jan 12, 2017 •

edited

Loading