Support truecolor in a terminal #6936

data-man · 2017-12-17T12:49:42Z

Currently for *nix only.
Example output (in Konsole):

Araq · 2017-12-17T23:35:31Z

I don't mind the feature but IMO terminal.nim should be as stateless as possible, setBackgroundColor and friends are a design invented before we got subroutine parameters.

data-man · 2017-12-17T23:46:28Z

Example's code:

import terminal

const Nim = "Efficient and expressive programming."

var
  fg = colYellow
  bg = colBlue
  int = 1.0

for i in 1..10:
  styledEcho bgColor, bg, fgColor, fg, Nim
  int -= 0.05
  fg = intensity(fg, int)

styledEcho resetStyle

@Araq
How can I improve the procs?

Araq · 2017-12-18T08:41:11Z

Hmm, my point was that styledEcho needs to support it but it looks like it already does. Maybe try to not export these?

data-man · 2017-12-18T17:07:13Z

I added the setForegroundColor*(stdout, color) and setBackgroundColor*(color: Color) templates only for consistency, because there is already setForegroundColor*(fg: ForegroundColor, bright = false), setBackgroundColor*(bg: BackgroundColor, bright=false) and others.

data-man · 2017-12-18T17:24:08Z

Maybe it's still useful to someone: Colours in terminal :)

Araq · 2017-12-18T23:37:55Z

If Windows does not support it, make it not compile on Windows, this should all be within an when defined(posix) then.

data-man · 2017-12-19T07:24:14Z

Windows support added. (Tested on Windows 10 in VirtualBox).

Araq · 2017-12-21T09:10:30Z

lib/pure/terminal.nim

+var
+  trueColorIsSupported = false
+  fgSetColor = true
+  colorsFGCache = initTable[Color, string]()


Breaks multi-threading.

Araq · 2017-12-21T09:10:55Z

lib/pure/terminal.nim

+from strutils import toLowerAscii
+import tables
+import colors
+export colors


I don't think terminal should export colors.

Araq · 2017-12-21T09:14:15Z

lib/pure/terminal.nim

+when defined(windows):
+  var mode: DWORD = 0
+  discard getConsoleMode(getStdHandle(STD_OUTPUT_HANDLE), addr(mode))
+  mode = mode or ENABLE_VIRTUAL_TERMINAL_PROCESSING


This is a breaking change ... Previously no escape sequences were interpreted.

Maybe to add (enable|disable|isSupported)TrueColor procs?

This is a breaking change, please don't do this at initialization but only if requested via enableTrueColor.

Araq · 2017-12-21T09:14:36Z

lib/pure/terminal.nim

+  trueColorIsSupported = setConsoleMode(getStdHandle(STD_OUTPUT_HANDLE), mode) != 0
+else:
+  when compileOption("taintmode"):
+    trueColorIsSupported = string(getEnv("COLORTERM")).toLowerAscii() in ["truecolor", "24bit"]


This version works for a disabled taint mode too.

Araq · 2017-12-21T09:14:58Z

lib/windows/winlean.nim

@@ -1059,6 +1059,28 @@ else:
                        lpNumberOfEventsRead: ptr cint): cint
       {.stdcall, dynlib: "kernel32", importc: "ReadConsoleInputW".}

+const


These are all terminal specific and so should be in terminal.nim.

data-man · 2017-12-21T14:38:36Z

Hanging a tests on AppVeyor after last commit :(

Araq · 2017-12-21T15:36:55Z

lib/pure/terminal.nim

@@ -677,3 +750,18 @@ when not defined(testing) and isMainModule:
  stdout.setForeGroundColor(fgBlue)
  stdout.writeLine("ordinary text")
  stdout.resetAttributes()
+
+fgSetColor = true
+colorsFGCache = initTable[Color, string]()


This only works for the main thread. ;-)

Araq · 2017-12-22T14:02:47Z

lib/pure/terminal.nim

+  styleCache = newTable[int, string]()
+
+fgSetColor = true
+colorsFGCache = newTable[Color, string]()


This only runs for the main thread!

Sorry I really do not understand what is wrong.

He means that the colorsFGCache will only be initialised in the main thread. If you try using it from another thread it will be nil and your code will likely fail.

Thanks!
I looked at the code in the /tests/parallel/tsendtwice.nim.
It works there. Or not?

data-man · 2017-12-27T12:57:58Z

Maybe disable caching when the thread mode is defined?

…ecolor

… term_truecolor

data-man · 2018-01-10T23:53:29Z

Corrections for Windows.
TrueColor disabled by default.

data-man · 2018-01-11T00:06:36Z

Hm, true color for Windows < 10 probably can be used with a non-standard terminals, e.g. ConEmu.

…ecolor

data-man · 2018-01-11T06:24:07Z

Added support for non-standard terminals.
Tested on Windows 8.1 with ConEmu.

Araq · 2018-01-11T08:14:57Z

changelog.md

+var
+  fg = colYellow
+  bg = colBlue
+  int = 1.0


a variable called 'int' that is of type float? Come on. ;-)

intensity :-)

This needs to be changed. Don't name your variables after types.

Araq · 2018-01-11T08:31:02Z

changelog.md

+  bg = colBlue
+  int = 1.0
+
+enableTrueColor()


This should be called enableTrueColors

…ecolor

johannschopplich · 2018-01-16T22:38:07Z

Quite looking forward to a merge on this pull request. 👍

Araq · 2018-01-18T07:59:41Z

lib/pure/terminal.nim

+
+proc setForegroundColor*(f: File, color: Color) =
+  ## Sets the terminal's foreground true color.
+  if trueColorIsEnabled:


Shouldn't this be assert(trueColorIsEnabled, "enableTrueColors() must be called before setForegroundColor")?

No. Because a true color may be used for debugging with a colored messages.

Araq · 2018-01-18T07:59:53Z

lib/pure/terminal.nim

+
+proc setBackgroundColor*(f: File, color: Color) =
+  ## Sets the terminal's background true color.
+  if trueColorIsEnabled:


Same question.

Same answer.

Araq · 2018-01-18T08:01:28Z

lib/pure/terminal.nim

+      styleCache: Table[int, string]
+
+var
+  trueColorIsSupported {.threadvar.}: bool


These threadvars are still not correct!

I can't understand: why a threadvars in other modules are correct?

Araq · 2018-01-18T08:02:12Z

lib/pure/terminal.nim

+fgSetColor = true
+
+when defined(windows):
+  import os


All this logic should be moved into enableTrueColors!

…ecolor

dom96 · 2018-01-18T16:39:44Z

lib/pure/terminal.nim

-    gFG = 0
-    gBG = 0
+    gFG {.threadvar.}: int
+    gBG {.threadvar.}: int


There is far too many global thread variables in this module (and this isn't your fault). But these should all be put into a single State object, eventually at least.

dom96 · 2018-01-18T16:44:10Z

lib/pure/terminal.nim

+    gFG {.threadvar.}: int
+    gBG {.threadvar.}: int
+
+  proc getStyleStr(style: int): string =


You're losing type safety here. The styleCache can be changed to an array[Style, string]

Araq reviewed Dec 21, 2017

View reviewed changes

data-man closed this Dec 21, 2017

data-man reopened this Dec 21, 2017

Araq reviewed Dec 22, 2017

View reviewed changes

yglukhov mentioned this pull request Dec 27, 2017

Fixed compilation error #6979

Merged

data-man added 11 commits December 27, 2017 20:45

support truecolor in a terminal

b084e79

fix

7b5662b

fix for taint mode

267049f

Add Windows support & colors cache

5aa2052

fix comments intendation

05cf084

threadvars

68261f9

trying fix

307a83d

use cache for styles too

6d47245

Fixing the prev fix ))

1080e24

Revert unittest change

dc1087d

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

b996328

…ecolor

data-man added 4 commits January 10, 2018 23:15

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

e293576

…ecolor

Correct checking for Windows

38d3340

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

c255bf8

…ecolor

Merge branch 'term_truecolor' of https://github.com/data-man/Nim into…

83d2bcf

… term_truecolor

Fix style const

24025af

data-man added 3 commits January 11, 2018 05:19

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

7e71675

…ecolor

Simplification

4c52244

Add support non-standard terminals

832e194

Araq reviewed Jan 11, 2018

View reviewed changes

data-man added 7 commits January 11, 2018 11:35

Renaming procs

a754ee1

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

cf3789c

…ecolor

fmt -> %

434ac18

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

77c2ca3

…ecolor

fmt <- %

39aeca3

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

6c61208

…ecolor

remove 'except %'

ee64271

Araq reviewed Jan 18, 2018

View reviewed changes

data-man added 2 commits January 18, 2018 11:14

Merge branch 'devel' of https://github.com/nim-lang/Nim into term_tru…

395eb0c

…ecolor

Removed an unnecessary assignment

ef5b0fc

dom96 reviewed Jan 18, 2018

View reviewed changes

Araq merged commit 7ce3812 into nim-lang:devel Jan 18, 2018

Support truecolor in a terminal #6936

Support truecolor in a terminal #6936

Conversation

data-man commented Dec 17, 2017

Araq commented Dec 17, 2017

data-man commented Dec 17, 2017

Araq commented Dec 18, 2017 • edited

data-man commented Dec 18, 2017

data-man commented Dec 18, 2017

Araq commented Dec 18, 2017

data-man commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

data-man commented Dec 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

data-man commented Dec 27, 2017

data-man commented Jan 10, 2018

data-man commented Jan 11, 2018

data-man commented Jan 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dom96 Jan 18, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johannschopplich commented Jan 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Araq commented Dec 18, 2017 •

edited

dom96 Jan 18, 2018 •

edited