Satisfy NO_COLOR requirements as per https://no-color.org

I replicated the same style as settings' and logger' found here—is that close to acceptable?

list

tui

added 1 commit

• 3df788ac - Respect the NO_COLOR environment variable (as per https://no-color.org)

Compare with previous version

I think there should be a more idiomatic way without unsafePerformIO. I'm not even sure if this triggers sharing properly despite NOINLINE. We can adjust app:

app :: App BrickState e String
app = App { appDraw         = \st -> [ui st]
          , appHandleEvent  = eventHandler
          , appStartEvent   = return
          , appAttrMap      = const defaultAttributes
          , appChooseCursor = neverShowCursor
          }

to be

app :: AttrMap -> App BrickState e String
...

And then do the env lookup in brickMain: https://gitlab.haskell.org/haskell/ghcup-hs/-/blob/master/app/ghcup/BrickMain.hs#L527

changed this line in version 3 of the diff

Same here. Here it's easier, since we're already in IO in printListResult, so lets do the lookup there.

changed this line in version 3 of the diff

I think this is a good idea. If you address the points I made I'm sure we can merge this quickly.

I can do those. The only reason I can think of for doing it in some kind of unsafePerformIO way is that if one uses color somewhere else it automatically complies with NO_COLOR. What do you think?

I want to get rid of unsafePerformIO, not add more ;)

added 1 commit

• a08e6243 - Respect NO_COLOR environment variable in list and tui

Compare with previous version

I think your suggestions made the patch look a lot better.

What is dimAttributes used for? It's probably the last missing piece of the puzzle: that blue bg needs to change to either nothing or reverse video depending on what this is.

It's used as an additional layer when a bindist isn't available for your distro (this happens on FreeBSD and Alpine). It's used here in ui function: https://gitlab.haskell.org/haskell/ghcup-hs/-/blob/e829bd82/app/ghcup/BrickMain.hs#L154

We can adjust

ui :: BrickState -> Widget String

...

app :: AttrMap -> App BrickState e String

to

ui :: AttrMap -> BrickState -> Widget String

...

app :: AttrMap -> AttrMap -> App BrickState e String

and pass it through similar to the other one

added 1 commit

• ead9d316 - Apply NO_COLOR to dimAttributes as well to cover all tui colors

Compare with previous version

I have tried to understand how dimming works by turning lNoBindist = True in all the Cabal entries as follows:

diff --git a/lib/GHCup.hs b/lib/GHCup.hs
index 0ac6e25..4abff9e 100644
--- a/lib/GHCup.hs
+++ b/lib/GHCup.hs
@@ -859,7 +859,7 @@ listVersions av lt criteria pfreq = do
       hlsPowered <- fmap (elem v) $ hlsGHCVersions
       pure ListResult { lVer = v, lCross = Nothing , lTag = tags, lTool = t, lStray = False, .. }
     Cabal -> do
-      let lNoBindist = isLeft $ getDownloadInfo Cabal v pfreq av
+      let lNoBindist = True -- isLeft $ getDownloadInfo Cabal v pfreq av
       lSet <- fmap (maybe False (== v)) $ cabalSet
       lInstalled <- cabalInstalled v
       pure ListResult { lVer    = v

Is there something wrong with dimAttributes, with my terminal, or is this the intended effect?

color

no color

I'm puzzled because the selection is still there and the keys do interact with them, but there is no visual clue.

I'll have to check. It's possible that this is terminal dependent.

Maybe we can add a section in the README e.g. here https://gitlab.haskell.org/haskell/ghcup-hs#configuration bout this env var

I'm also planning to explain the other environment variables there.

This is what happens with dimming and NO_COLOR. Is that what's intended? gray colors shades are allowed per the NO_COLOR "spec" right?

I'm also thinking we should retain:

  , ("hooray"       , Vty.defAttr `withForeColor` Vty.brightWhite)

even if NO_COLOR is set... it marks new versions in bright white

Thanks for testing it! Your terminal's version of dimming is what I had expected.

My understanding of what's allowed and what isn't is just the "simple" test: if it uses a color escape code, it's out, if not, it's fine.

I ventured into the vty source and I think I got to some kind of demarkation. The styles italic, standout, underline, reverse video, blink, dim, and bold are "capabilities" and the terminfo implementation probes for them. Those in my opinion are fair game but I did shoot the NO_COLOR guy a message to hear what he thinks as well.

Assuming that's close to how this all works I'd rather use standout or bold in place of brightWhite depending of what works best.

Update: it's greenlit, I got back:

i am not opposed to those, and it is how most of my software is configured

if the DEC VT* series supported them, i say use them :)

P.S. can you also show me how it looks for you when you're focused on a dimmed entry? I'm guessing the same bg as the focused line but w/ dimmed chars

I can't really find style that is a good substitute for brightWhite foreground. Vty.bold does nothing on my terminal, standout conflicts with reverseVideo and blink does nothing either.

P.S. can you also show me how it looks for you when you're focused on a dimmed entry? I'm guessing the same bg as the focused line but w/ dimmed chars

You cannot focus on a dimmed entry currently, the blue bar disappears... maybe that needs to be fixed as well.

I don't like CSS and styling in general. It's all irrational and unstructured. Suggestions welcome.

Sorry, does that answer your questions? Is there anything else we need to do?

merged