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

Refactor all git calls to capture STDERR properly #183

Closed
wants to merge 15 commits into from

Conversation

mnaoumov
Copy link

Fixes #182

@@ -299,6 +299,7 @@ if (Test-Path Function:\TabExpansion) {
}

function TabExpansion($line, $lastWord) {
"line: $line, lastWord: $lastWord" > c:\dev\333.txt
Copy link
Owner

Choose a reason for hiding this comment

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

Debugging?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@dahlbyk
Copy link
Owner

dahlbyk commented May 14, 2015

Sorry for the delay in looking at this - really nice work! I'm going to give this a try locally, but everything makes sense at a glance.

Can you add a note to Exec.ps1 referencing https://github.com/mnaoumov/Invoke-NativeApplication for anyone looking for more detail as to what's going on there?

@@ -133,7 +133,7 @@ function Write-GitStatus($status) {
Write-Prompt $s.AfterText -BackgroundColor $s.AfterBackgroundColor -ForegroundColor $s.AfterForegroundColor

if ($WindowTitleSupported -and $s.EnableWindowTitle) {
if( -not $Global:PreviousWindowTitle ) {
if (-not (Test-Path -Path Variable:Global:PreviousWindowTitle)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you apply this strict revision to elseif ( $Global:PreviousWindowTitle ) below? Getting this:

The variable '$Global:PreviousWindowTitle' cannot be retrieved because it has not been set.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@dahlbyk
Copy link
Owner

dahlbyk commented May 14, 2015

Cool, fixes look good. Going to run with this for today and see if anything seems out of sorts.

@dahlbyk
Copy link
Owner

dahlbyk commented Dec 31, 2016

@rkeithhill @lzybkr what do you think about this PR?

@rkeithhill
Copy link
Collaborator

Take a look at my comment on the related issue - #182. With 0.6.1, I can't repro the issue this PR is meant to fix. Can you?

I simply see the <SHA1>... in the detached HEAD state. It also inserts safeexec in places that weren't ignoring stderr before and the TestInPrompt function seems a bit brittle, relying on index 2 up the PS callstack. And I'm also not a big fan of using aliases in a script. That is just one more level of indirection to lookup. :-)

@dahlbyk
Copy link
Owner

dahlbyk commented Jan 2, 2017

Yeah, I think we're probably better off with targeted error handling (or, as in #307, using Git options to suppress errors).

Still, belated thanks for the contribution @mnaoumov!

@dahlbyk dahlbyk closed this Jan 2, 2017
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 this pull request may close these issues.

Posh-git cmdlets misbehave if you have $ErrorActionPreference = "Stop"
3 participants