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

WIP allow prompt char > to reflect nesting level. #363

Merged
merged 1 commit into from
Jan 8, 2017

Conversation

rkeithhill
Copy link
Collaborator

To be a good prompt citizen in PowerShell, the > (or whatever prompt char) should be displayed once for nesting level 0, twice for nesting level 1, and so on.

In order to make that happen, we have to allow for deferred expansion of the Prompt*Suffix settings.

See https://msdn.microsoft.com/en-us/powershell/reference/5.1/microsoft.powershell.core/about/about_prompts for more details.

To be a good prompt citizen in PowerShell, the > (or whatever prompt char) should be displayed once for nesting level 0, twice for nesting level 1, and so on.

In order to make that happen, we have to allow for deferred expansion of the Prompt*Suffix settings.

See https://msdn.microsoft.com/en-us/powershell/reference/5.1/microsoft.powershell.core/about/about_prompts for more details.
@@ -38,7 +38,7 @@ $poshGitPromptScriptBlock = $null

$currentPromptDef = if ($funcInfo = Get-Command prompt -ErrorAction SilentlyContinue) { $funcInfo.Definition }

# HACK: If prompt is missing, create a global one we can overwrite with Set-Item
# HACK: If prompt is missing, create a global one we can overwrite with Set-Item
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sometimes GitHub diff gets a little whacked. I didn't change that line at all.

Copy link
Owner

Choose a reason for hiding this comment

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

Apparently I committed a LF here instead of CRLF:
notepad _2017-01-08_00-32-42

VS Code is fixing it. #336 will prevent this type of change in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I was a bit puzzled on that one.

@@ -85,7 +85,7 @@ if (!$currentPromptDef -or ($currentPromptDef -eq $defaultPromptDef)) {
}

$global:LASTEXITCODE = $origLastExitCode
$promptSuffix
$ExecutionContext.SessionState.InvokeCommand.ExpandString($promptSuffix)
Copy link
Owner

Choose a reason for hiding this comment

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

TIL

I probably would have done something way more involved like check if $promptSuffix was a [ScriptBlock] to execute every time.

Copy link
Owner

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

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

LGTM. I assume ExpandString is available in PS v2?

@dahlbyk
Copy link
Owner

dahlbyk commented Jan 8, 2017

WIP allow prompt char ...

Commit message style note: WIP PRs are 100% approved, but generally I would avoid "WIP" in commit messages unless you're planning to rewrite history before we actually review and merge the code. Which I'm totally fine with (in fact, clean history is encouraged!), if we communicate as such. Unclear if that's your intent here, so thought I'd mention it.

@rkeithhill
Copy link
Collaborator Author

I assume ExpandString is available in PS v2?

Yes, I tested on v2 to make sure.

I would avoid "WIP" in commit messages unless you're planning to rewrite history

So don't use WIP in commit msg unless it really is WIP? :-) Sorry about that. I thought it would change because I was thinking of replacing Prompt*Suffix with PromptChar. But I've changed my mind since.

@rkeithhill rkeithhill merged commit a1795ab into master Jan 8, 2017
@rkeithhill rkeithhill deleted the rkeithhill/nestedPromptLevel-tweak branch January 8, 2017 07:35
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.

2 participants