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

Enhance PoshGitTextSpan support for custom VT seqs #616

Merged
merged 6 commits into from
Feb 18, 2019

Conversation

rkeithhill
Copy link
Collaborator

@rkeithhill rkeithhill commented Sep 4, 2018

@dahlbyk See what you think about this. In the process of explaining for #612 how to use the CustomAnsi property on PoshGitTextSpan to customize built-in prompt, I realized my previous impl of this idea was too limiting. For instance before the Path is display the user wanted this displayed in the prompt:

image

The ideal property to use for this info is the $GitPromptSettings.DefaultPromptPrefix but there are four different bits of info displayed with different colors. That currently does NOT work.

My original idea with CustomAnsi was that folks could specify a custom sequence that would apply to the text. That would allow the user to set other attributes like say underline or inverse as well as fg/bg colors. But that sequence would apply to all of the text so you could have only a single span of text with those attributes.

Now I'm thinking perhaps we should just ditch the CustomAnsi property and allow folks to put as much custom seqs in the Text field as their heart desires. For instance, with this change, I can achieve the above desired display with this one setting:

function Test-Administrator { $true }

$GitPromptSettings.DefaultPromptPrefix.Text =
    "`n`$(Get-PromptConnectionInfo -Format `"[{1}@{0}]: `")" +
    "`$(if (global:Test-Administrator) {`"$([char]27)[37m^`"})" +
    "$([char]27)[33m`${env:USERNAME}@$([char]27)[95m`${env:COMPUTERNAME}$([char]27)[90m:"

Thoughts?

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.

Now I'm thinking perhaps we should just ditch the CustomAnsi property and allow folks to put as much custom seqs in the Text field as their heart desires.

👍

src/AnsiUtils.ps1 Show resolved Hide resolved
@rkeithhill
Copy link
Collaborator Author

I'll fix the tests tonight. First, I wanted to see if you agreed in principle to this change.

@rkeithhill
Copy link
Collaborator Author

Oh yeah baby, fixed the AppVeyor tests. I submit a separate PR for that.

@rkeithhill rkeithhill force-pushed the rkeithhill/enhance-customansi branch from d064df1 to 49557be Compare October 4, 2018 05:45
@rkeithhill rkeithhill changed the title WIP: Enhance PoshGitTextSpan support for custom VT seqs Enhance PoshGitTextSpan support for custom VT seqs Oct 4, 2018
@rkeithhill
Copy link
Collaborator Author

rkeithhill commented Oct 4, 2018

@dahlbyk This is ready for a final review/merge.

@rkeithhill
Copy link
Collaborator Author

rkeithhill commented Oct 20, 2018

One thing I wonder about the current impl is that if the user embeds VT seqs in the .Text property without a terminating seq, we write that string "as-is" when AnsiConsole is set to $false. That can mess up the screen colors. In Write-Prompt we could A) strip out seqs, B) add a terminating seq if we detect seqs or C) leave it as it is. If the user directly evals .Text then there's nothing we can do.

@rkeithhill rkeithhill merged commit be45638 into master Feb 18, 2019
@rkeithhill rkeithhill deleted the rkeithhill/enhance-customansi branch February 18, 2019 03:10
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