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

Do not update WindowTitle at all in prompt func if setting is $null #597

Merged
merged 5 commits into from
Jul 12, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 22 additions & 37 deletions src/posh-git.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ if (!$Env:HOME) { $Env:HOME = "$Env:USERPROFILE" }
$IsAdmin = Test-Administrator

# Probe $Host.UI.RawUI.WindowTitle to see if it can be set without errors
$WindowTitleSupported = $false
$HostSupportsSettingWindowTitle = $false
Copy link
Owner

Choose a reason for hiding this comment

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

If we're going to let $GitPromptSettings.WindowTitle = $false really suppress touching WindowTitle, we might as well avoid this, too.

Thoughts on moving this whole block into a Test-WindowTitle function (in Utils.ps1)? I figure that function would close over a script variable that's $null until we actually need to probe, at which point it would probe and remember for future checks.

The check below would then be:

if ($settings.WindowTitle -and (Test-WindowTitle)) {
    # ...

This would short-circuit the probe until $settings.WindowTitle isn't falsey.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But when $settings.WindowTitle isn't falsey, we would be doing this "writable" check during every prompt eval - effectively setting the title three times.

I think it is better to do this check once during module load but I'm fine with creating a function in Utils.ps1 so we'd have:

$IsAdmin = Test-Administrator
$IsWindowTitleWritable = Test-WindowTitleIsWritable

try {
$global:PreviousWindowTitle = $Host.UI.RawUI.WindowTitle
$newTitle = "${global:PreviousWindowTitle} "
$global:PoshGitOrigWindowTitle = $Host.UI.RawUI.WindowTitle
Copy link
Owner

Choose a reason for hiding this comment

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

Does this actually need to be global? Could it be script-scoped alongside $HostSupportsSettingWindowTitle, with a corresponding Reset-WindowTitle to use in OnRemove?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it could be script scoped.

$newTitle = "${global:PoshGitOrigWindowTitle} "
$Host.UI.RawUI.WindowTitle = $newTitle
$WindowTitleSupported = ($Host.UI.RawUI.WindowTitle -eq $newTitle)
$Host.UI.RawUI.WindowTitle = $global:PreviousWindowTitle
$HostSupportsSettingWindowTitle = ($Host.UI.RawUI.WindowTitle -eq $newTitle)
$Host.UI.RawUI.WindowTitle = $global:PoshGitOrigWindowTitle
}
catch {
Write-Debug "Probing for WindowTitleSupported errored: $_"
$global:PoshGitOrigWindowTitle = $null
Write-Debug "Probing for HostSupportsSettingWindowTitle errored: $_"
}

# Get the default prompt definition.
Expand All @@ -43,10 +44,6 @@ else {
$GitPromptScriptBlock = {
$settings = $global:GitPromptSettings
if (!$settings) {
if ($WindowTitleSupported -and $global:PreviousWindowTitle) {
$Host.UI.RawUI.WindowTitle = $global:PreviousWindowTitle
}

return "<`$GitPromptSettings not found> "
}

Expand Down Expand Up @@ -92,34 +89,22 @@ $GitPromptScriptBlock = {
$promptSuffix.Text = $promptSuffix.Text.Substring(0, $promptSuffix.Text.Length - 1)
}

# Update the host's WindowTitle is host supports it and user has not disabled $GitPromptSettings.WindowTitle
# Update the host's WindowTitle if host supports it and user has not disabled $GitPromptSettings.WindowTitle
# This has to be *after* the call to Write-VcsStatus, which populates $global:GitStatus
if ($WindowTitleSupported) {
$windowTitle = $settings.WindowTitle
if (!$windowTitle) {
if ($global:PreviousWindowTitle) {
$Host.UI.RawUI.WindowTitle = $global:PreviousWindowTitle
if ($HostSupportsSettingWindowTitle -and $settings.WindowTitle) {
Copy link
Owner

Choose a reason for hiding this comment

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

While we're at it, should this whole block move into a Set-WindowTitle function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, but we'd need to export it from the module to make it callable from the prompt func. How about Reset-WindowTitle - does it need to be exported or would we be the only caller in the OnRemove event handler.

try {
if ($settings.WindowTitle -is [scriptblock]) {
$windowTitleText = & $settings.WindowTitle $global:GitStatus $IsAdmin
}
}
else {
try {
if ($windowTitle -is [scriptblock]) {
$windowTitleText = & $windowTitle $global:GitStatus $IsAdmin
}
else {
$windowTitleText = $ExecutionContext.SessionState.InvokeCommand.ExpandString("$windowTitle")
}

# Put $windowTitleText in a string to ensure results returned by scriptblock are flattened to a string
$Host.UI.RawUI.WindowTitle = "$windowTitleText"
else {
$windowTitleText = $ExecutionContext.SessionState.InvokeCommand.ExpandString("$($settings.WindowTitle)")
}
catch {
if ($global:PreviousWindowTitle) {
$Host.UI.RawUI.WindowTitle = $global:PreviousWindowTitle
}

Write-Debug "Error occurred during evaluation of `$GitPromptSettings.WindowTitle: $_"
}
# Put $windowTitleText in a string to ensure results returned by scriptblock are flattened into a string
$Host.UI.RawUI.WindowTitle = "$windowTitleText"
}
catch {
Write-Debug "Error occurred during evaluation of `$GitPromptSettings.WindowTitle: $_"
}
}

Expand Down Expand Up @@ -171,9 +156,9 @@ if ($ForcePoshGitPrompt -or !$currentPromptDef -or ($currentPromptDef -eq $defau
$ExecutionContext.SessionState.Module.OnRemove = {
$global:VcsPromptStatuses = $global:VcsPromptStatuses | Where-Object { $_ -ne $PoshGitVcsPrompt }

# Revert original WindowTitle
if ($WindowTitleSupported -and $global:PreviousWindowTitle) {
$Host.UI.RawUI.WindowTitle = $global:PreviousWindowTitle
# Revert original WindowTitle but only if posh-git is currently configured to set it
if ($HostSupportsSettingWindowTitle -and $global:GitPromptSettings.WindowTitle -and $global:PoshGitOrigWindowTitle) {
$Host.UI.RawUI.WindowTitle = $global:PoshGitOrigWindowTitle
}

# Check if the posh-git prompt function itself has been replaced. If so, do not restore the prompt function
Expand Down