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

[RFC] Ability to disable and enable colouring #26

Open
1 task
chshersh opened this issue Apr 19, 2020 · 3 comments · Fixed by #38
Open
1 task

[RFC] Ability to disable and enable colouring #26

chshersh opened this issue Apr 19, 2020 · 3 comments · Fixed by #38
Assignees
Labels
question Further information is requested

Comments

@chshersh
Copy link
Contributor

chshersh commented Apr 19, 2020

It would be nice if colourista could support automatic disabling and enabling of colouring, so we can easily disable/enable colours in our code. We want to have a solution that satisfies the following requirements:

  1. Allows specifying the colour only in a single place in the application.
  2. Colours the text by default, if no option is specified.
  3. Doesn't involve code duplication.
  4. Doesn't require code recompilation to change colouring settings.

I'll describe one possible solution below.

-XImplicitParams

One way to do this is to use the ImplicitParams Haskell language extension. It contains the following parts:

  1. Implement simple enum to control colouring mode.
    data ColourMode = EnableColour | DisableColour
  2. Introduce a constraint with implicit params:
    type HasColourMode = (?colourMode :: ColourMode)
  3. Patch each function to pattern-math on a colour:
    withColourMode :: (HasColourMode, IsString  str) => str -> str
    withColourMode = case ?colourMode of
        EnableColour -> str
        DisableColour -> ""
    
    red :: (HasColourMode, IsString str) => str
    red = withColourMode $ fromString $ setSGRCode [SetColor Foreground Vivid Red]
    {-# SPECIALIZE red :: HasColourMode => String     #-}
    {-# SPECIALIZE red :: HasColourMode => Text       #-}
    {-# SPECIALIZE red :: HasColourMode => ByteString #-}
  4. Implement magic instance described in this blog post to make EnableColour default:
    -- ?color = EnableColor
    instance IP "colourMode" ColourMode where
        ip = EnableColour

Cons and pros

Implementation cost: patch each function.

Pros:

  1. All existing code should work without any changes.
  2. We can easily disable/enable colouring by defining a single variable in a single module after we parse --no-colour option or something like this.

Cons

  1. Each function that performs colouring or calls colouring function should add HasColourMode constraint.
  2. You don't have compile-time guarantees if you forget to add such constraint somewhere.
  3. This involves using non-common GHC feature.

Additionally we need to add tests for this:

  • Check that disabling colour mode actually disables colour codes
@chshersh chshersh added the question Further information is requested label Apr 19, 2020
@chshersh
Copy link
Contributor Author

chshersh commented Jun 4, 2020

An alternative solution is to use the reflection for this purpose. It's similar to ImplicitParams. I've spent some time diving into this library, and I'm going to describe the approach and provide a comparison with ImplicitParams.

How to implement?

  1. Add the reflection library to dependencies.
  2. Implement simple enum for colouring mode (same as in the previous approach):
    data ColourMode = EnableColour | DisableColour
  3. Helper function:
    withColourMode :: (Reifies s ColourMode, IsString  str) => str -> str
    withColourMode = case (reflect $ Proxy @mode) of
        EnableColour -> str
        DisableColour -> ""
  4. Use helper function this:
formatWith
    :: (Reifies s ColourMode, IsString str, Semigroup str)
    => [str]
    -> str
    -> str
formatWith formatting str = case formatting of
    []   -> str
    x:xs -> withColourMode @s (sconcat (x :| xs)) <> str <> withColourMode @s reset

Costs of implementation

  • Pros: 🤷
  • Cons
    • Extra dependency
    • SPECIALIZE doesn't work anymore
    • Requires to use AllowAmbiguousTypes ann pass the parameter explicitly actually
    • Still not clear how to provide the initial ColourMode and pass it implicitly through

Conclusion

After looking at the reflection package, I came to the conclusion that its primary usage is constraint of some typeclasses where you want typeclasses to depend on some runtime data. This is not our case, so ImplicitParams should work just fine 👍

Alternative ideas

We can skip step 4 in the solution with the ImplicitParams extensions, so each function that uses colouring or calls a function that uses colouring must specify HasColourMode constraint. With this approach, some code needs to be changed (need to try on some project to see how much code needs to be changed, hard to estimate), but we also gain some extra type-safety and guarantees that our explicitly specified value of ColourMode actually will be used.

@chshersh chshersh self-assigned this Jun 4, 2020
@vrom911
Copy link
Member

vrom911 commented Jun 4, 2020

Thanks for such detailed analysis and overview, @chshersh ! 🧠 🎉
I would definitely like to try the ImplicitParameters options first with the test phase on an actual (and preferably not the smallest one). The plan for that is very clear already 👍
I am a bit sceptical as well about the reflection library in our use case, seems like the cost is much bigger.
Btw, we can continue documenting decisions under this issue, it is super helpful 🙏

chshersh added a commit that referenced this issue Aug 21, 2020
@chshersh chshersh added this to the v0.2.0.0: Colour modes milestone Aug 21, 2020
chshersh added a commit that referenced this issue Aug 24, 2020
vrom911 pushed a commit that referenced this issue Aug 24, 2020
* [#26] Ability to disable and enable colouring

Resolves #26

* Fix build on old GHCs

* [#26] Move ColourMode to separate module, improve docs

Resolves #26
chshersh added a commit that referenced this issue Apr 5, 2021
@chshersh chshersh reopened this Apr 5, 2021
@chshersh chshersh removed this from the v0.2.0.0: Colour modes milestone Apr 5, 2021
@chshersh
Copy link
Contributor Author

chshersh commented Apr 5, 2021

The implementation was reverted due to a bug in GHC and not reliable behaviour.

vrom911 pushed a commit that referenced this issue 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 a pull request may close this issue.

2 participants