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 Data Strings & Show-MeditationPrompt #126

Merged
merged 3 commits into from
Jan 11, 2019
Merged

Refactor Data Strings & Show-MeditationPrompt #126

merged 3 commits into from
Jan 11, 2019

Conversation

vexx32
Copy link
Owner

@vexx32 vexx32 commented Jan 10, 2019

New design:
image

Also moved all data strings to PSD1 data file for cleanliness.

✨ Added some extra color to the koan quotes!
🔧 Fix sporadic class import issues
@vexx32 vexx32 self-assigned this Jan 10, 2019
@vexx32 vexx32 added PR-Needs-Review 🔍 Let's take a closer look! Category-Module Pertaining to the module's functionality itself. labels Jan 10, 2019
@vexx32 vexx32 added this to the v0.43.0 milestone Jan 10, 2019
@vexx32
Copy link
Owner Author

vexx32 commented Jan 10, 2019

Failing build is due to issue with BuildHelpers module used for CI. Looks like @RamblingCookieMonster has been tinkering around, and for whatever reason one of the build variables isn't able to be bound to a string despite confirming it's not null. One of these sections seems to be the culprit (Error talks about parameter binding exception for a -Value parameter).

https://github.com/RamblingCookieMonster/BuildHelpers/blob/fc2d8409f3285fa790b1666dbcfd4619dd107603/BuildHelpers/Public/Set-BuildEnvironment.ps1#L118-L124

https://github.com/RamblingCookieMonster/BuildHelpers/blob/fc2d8409f3285fa790b1666dbcfd4619dd107603/BuildHelpers/Public/Set-BuildEnvironment.ps1#L134

https://github.com/RamblingCookieMonster/BuildHelpers/blob/fc2d8409f3285fa790b1666dbcfd4619dd107603/BuildHelpers/Public/Set-BuildEnvironment.ps1#L135-L137

vexx32 referenced this pull request in RamblingCookieMonster/BuildHelpers Jan 10, 2019
@thomasrayner
Copy link
Contributor

@vexx32 Have you considered manipulating the background color in addition to the foreground color? Some of your choices might look a little wonky for folks using different themes (like some of the high contrast ones).

@vexx32
Copy link
Owner Author

vexx32 commented Jan 10, 2019

@thomasrayner I've considered it, but taking into account the fact that those colors may also be modified from the defaults arbitrarily, I'm not sure what I can do in terms of making sure it remains readable?

@SeeminglyScience
Copy link

@vexx32 If PSReadLine is loaded you can pull from it's settings to determine what colors to use.

$psrlModule = Get-Module PSReadLine
if (-not $psrlModule) {
    return
}

$options = Get-PSReadLineOption
if ($psrlModule.Version.Major -ge 2) {
    # Handle as ANSI escape
    return '{1}Colored like a number{0} {2}Colored like a keyword{0}' -f (
        "$([char]27)[0m", # Reset color ANSI escape
        $options.NumberColor,
        $options.KeywordColor)
} else {
    # Handle like ConsoleColor
}

@vexx32
Copy link
Owner Author

vexx32 commented Jan 10, 2019

That... is complicated. But I appreciate the pointer! I'll copy it to an issue so I can keep track of it, but it probably won't happen anytime soon 😕

@thomasrayner
Copy link
Contributor

Maybe in the meantime, it would be easier to implement a Set-PSKoansColorScheme -Mode <fancy | consoledefault> for people to opt in and out

@vexx32
Copy link
Owner Author

vexx32 commented Jan 10, 2019

🤔

Yeah, I can see why you'd need that, I suppose. I'll think on it. 😄

@thomasrayner
Copy link
Contributor

🤔

Yeah, I can see why you'd need that, I suppose. I'll think on it. 😄

Just to eliminate all the colors in general and use whatever bland colors the console is set to. Maybe colorblind people have a setting they like and are annoyed by the fanciness 🤷‍♂️

@vexx32
Copy link
Owner Author

vexx32 commented Jan 10, 2019

See #127. Yep, the option will be there... sooner or later 😉

@vexx32 vexx32 merged commit eac7d82 into master Jan 11, 2019
@vexx32 vexx32 deleted the RefactorData branch January 11, 2019 03:24
@vexx32 vexx32 removed the PR-Needs-Review 🔍 Let's take a closer look! label Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category-Module Pertaining to the module's functionality itself.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants