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: ANSI and settings prototype #456

Closed
wants to merge 19 commits into from

Conversation

rkeithhill
Copy link
Collaborator

@rkeithhill rkeithhill commented Mar 5, 2017

I started this about a week ago to experiment with settings and ANSI seq support. I like the use of C# source to expose types to the end user. I originally started with PS v5 classes which I left in temporarily (IGNORE AnsiUtils.ps1) but I ran into issues like exposing the types to the end user.

This approach introduces a TextSpan class that can simplify the GitPromptSettings a bit by combining text, fg and bg together into one class. Need to decide how to handle some of the settings that also have a bright fg color.

I have implemented but not tested xterm/24-bit color support but need to test it. Anyway, maybe we can take the good parts from each approach.

@rkeithhill
Copy link
Collaborator Author

OK did a wee bit of testing on the 24-bit color support on Windows PSCore and xterm256 testing on Ubuntu. Other than that annoying PSReadline double-prompt bug this seems to be working. There are some rough edges with creating a Color that I need to improve. For instance, to get the correct Color ctor for xterm you have to do this:

$GitPromptSettings.BranchIdenticalStatusSymbol.ForegroundColor = [PoshGit.Color]::new(([byte]96))

in order to distinguish from RGB that is specified as an int:

$GitPromptSettings.BranchIdenticalStatusSymbol.ForegroundColor = [PoshGit.Color]::new(96)

One option would be to require the ColorMode for xterm e.g.:

$GitPromptSettings.BranchIdenticalStatusSymbol.ForegroundColor = [PoshGit.Color]::new(96, [ColorMode]::XTerm256)

@rkeithhill rkeithhill requested a review from dahlbyk March 6, 2017 04:02
@dahlbyk
Copy link
Owner

dahlbyk commented Mar 6, 2017

Haven't had a chance to review the code yet, but...

Need to decide how to handle some of the settings that also have a bright fg color.

I don't think these need to be represented in the settings. We can initialize with bright defaults based on current RawUI settings, but customize though the normal setting.


I have been pondering if it would be easier to skip Text/FG/BG in favor of the settings simply being strings that can include ANSI color sequences (generated using an exported function or an ANSI module dependency).

@rkeithhill
Copy link
Collaborator Author

rkeithhill commented Mar 6, 2017

@lzybkr I would appreciate if you could take a quick peek at this. I'm wondering about the perf implications of using Add-Type to compile .cs files vs Add-Type C# verbatim strings in a .ps1 file. It's kind of nice to be able to edit the C# files and get syntax highlighting and error squiggles. Import perf on Windows Desktop is pretty good but on PS Core, import takes between 1 and 2 seconds.

I guess we could always precompile the native bits into a portable DLL and that should alleviate perf issues on PS Core.

@rkeithhill
Copy link
Collaborator Author

I don't think these need to be represented in the settings.

Good because I felt a bit icky adding that property to the TextSpan class. :-)

@lzybkr
Copy link
Collaborator

lzybkr commented Mar 6, 2017

Add-Type with cs files is probably better than a string argument for overall performance, that large string wouldn't get garbage collected as a string in PowerShell.

Add-Type was especially slow in PowerShell Core because Roslyn wasn't cross-gen'd - but it should be now, check if you have the latest release.

@rkeithhill
Copy link
Collaborator Author

@lzybkr Thanks! I have PS Core Alpha 16. Or are you referring to the version of dotnet?

@lzybkr
Copy link
Collaborator

lzybkr commented Mar 6, 2017

Hmm, I was thinking of this PR which is in alpha.16.

@lzybkr lzybkr closed this Mar 6, 2017
@lzybkr lzybkr reopened this Mar 6, 2017
The handling for DarkMagenta happens now by just assigning former FgBrightColors to the corresponding FgColor *if* the bg is DarkMagenta.  This isn't dynamic anymore but the user can always change the fg colors.
The updates work for a "class" instead of a "struct" where you don't have to worry about comparing with null or an instance being null.
@rkeithhill
Copy link
Collaborator Author

I'm going to abandon this PR in favor of #513

@rkeithhill rkeithhill closed this Jan 1, 2018
@rkeithhill rkeithhill deleted the rkeithhill/settings-prototype branch May 14, 2018 03:37
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.

3 participants