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

[#26] Ability to disable and enable colouring #38

Merged
merged 3 commits into from Aug 24, 2020

Conversation

chshersh
Copy link
Contributor

@chshersh chshersh commented Aug 21, 2020

Resolves #26

Implemented using Implicit parameters. Verified in GHCi that it works!

Screenshot from 2020-08-21 17-52-07

Ideally, all existing code that uses colourista can simply upgrade to the latest version and it should work without any changes. If users want to support disabling and enabling of colouring, they need to add explicit HasColourMode constraint. Currently, we have a magic instance that automatically sets colour mode to EnableColour. But in the future, we can remove it, once users migrate to this new version, so we can have compile-time guarantees on making sure this constraint is enabled.

Open questions (and possible TODOs):

  • Move ColourMode into a separate module like Colourista.Mode?
  • Add some more docs?

@chshersh chshersh requested a review from vrom911 as a code owner August 21, 2020 16:50
@chshersh chshersh self-assigned this Aug 21, 2020
@chshersh chshersh added the question Further information is requested label Aug 21, 2020
Copy link

@hint-man hint-man bot left a comment

Choose a reason for hiding this comment

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

There is no place for me here... I will choose the truth I like.

Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

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

Wow, this is some kind of magic 🧙🏼‍♂️
I really like how simple the functions still look!
I agree, that we should move in into a separate module.
Also, regarding the docs, I think that it would be helpful to have:

  • Migration guides (even if you don't need to change anything, I still think it should be added explicitly)
  • Explanation on how implicit params work, what does ? mean there and some links to the docs would be awesome
  • Also updated README and .cabal file description with the note about this feature and probably example?

Generally, that is a really fantastic feature, and you come up with am awesome solution 👏🏼 ❤️

Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

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

Fantastic work! 👏🏼 🏆

@vrom911 vrom911 merged commit d2bcd11 into master Aug 24, 2020
@vrom911 vrom911 deleted the chshersh/26-RFC-Ability-to-disable-and branch August 24, 2020 17:14
chshersh added a commit that referenced this pull request Apr 5, 2021
vrom911 pushed a commit that referenced this pull request Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Ability to disable and enable colouring
2 participants