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.

Sign up for GitHub

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

Jump to bottom

Move ColorSystem into Capabilities #343

Closed
wischi-chr opened this issue Apr 4, 2021 · 0 comments · Fixed by #372
Closed

Move ColorSystem into Capabilities #343

wischi-chr opened this issue Apr 4, 2021 · 0 comments · Fixed by #372
Labels
feature

Comments

@wischi-chr
Copy link
Contributor

Is your feature request related to a problem? Please describe.
It's not a problem with the behavior but with how the Profile is currently structured.

Describe the solution you'd like
The ColorSystem property should be moved to the Capabilities class because it really represents one of the capabilities of the console.

The class RenderContext makes this even more evident. Because ColorSystem is not inside Capabilities / IReadOnlyCapabilities it results in a (IMHO) strange egde case where it has to be passed in as a separate constructor parameter and stored in a separate property inside RenderContext.

Describe alternatives you've considered
The alternative would be to leave it as it is and I have to live with the fact that it's not implemented how I would do it 😅
@patriksvensson patriksvensson mentioned this issue Apr 13, 2021
Merged
phil-scott-78 pushed a commit that referenced this issue Apr 14, 2021
@patriksvensson @phil-scott-78
Add output abstraction and reorganize profile
3e2eea7 
* Moves ColorSystem from Profile to Capabilities
* Renames Tty to IsTerminal
* Adds IAnsiConsoleOutput to make output more flexible

Closes #343
Closes #359
Closes #369
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add output abstraction and reorganize profile
2 participants
@patriksvensson @wischi-chr