paulscherrerinstitute / StreamDevice

EPICS Driver for message based I/O
GNU General Public License v3.0
28 stars 42 forks source link

Allow configuring of coloured console output #64

Closed FreddieAkeroyd closed 3 years ago

FreddieAkeroyd commented 3 years ago

The STREAM_DEBUG_COLOR environment variable can be set to: yes - always generate ANSI colour escape codes on debug/error output no - use plain text output for debug/error auto - (default) output ANSI codes if terminal supports it

FreddieAkeroyd commented 3 years ago

Should I read STREAM_DEBUG_COLOR on every call to ansiEscape()?

dirk-zimoch commented 3 years ago

Hi Freddie, Thanks for your work. Looks good. Reading the environment variable every time may imply some severe overhead. But I have not tested that. At least the glibc implementation uses a linear search over all environment variables each time. (https://code.woboq.org/userspace/glibc/stdlib/getenv.c.html). I would only do that each time the output file changes (-> streamSetLogfile). Maybe that function should be relocated to StreamError.cc?

FreddieAkeroyd commented 3 years ago

I think the output file referred to by streamSetLogfile only gets sent non-coloured output? As far as I could tell coloured output was only being sent to stdout/stderr, but maybe there is an indirect mechanism?

As well as the environment variable there is also checking if output is to console or file:

dirk-zimoch commented 3 years ago

You may be right. It is so long ago that I last looked at the code. In that case, I do not expect stout to change at run-time thus checking isatty(stdout) once should be sufficient. And it is stdout, not epicsGetStdout() where the output is printed to (I think). I would rather keep EPICS out of the generic code like StreamError.

If the user only ever sets STREAM_DEBUG_COLOR in the startup script before calling iocInit, reading the variable once should be fine. But for changing it at run-time it may be better to have a global variable accessible from iocsh, e.g var streamDebugColor. That would only cost execution time when changing it but not when reading it. In that case an environment variable may be redundant. the iocsh variable can be a boolean initialized with isatty(stdout).

FreddieAkeroyd commented 3 years ago

Yes, an iocsh meachanism sounds a good route. I think IOC variables can only be int/double type so we can either use some integer values (0=auto,1=yes, 2=no) or make streamDebugColor a function instead and use streamDebugColor("auto") in ioc

dirk-zimoch commented 3 years ago

We don't need auto when we initialize with isatty(stdout), because that is auto. The user can then set it to 0 to 1. Setting it to auto at run-time makes not much sense.

FreddieAkeroyd commented 3 years ago

Yes, you are right - we don't re-check stdout so auto as an option doesn't make much sense. I'd been thinking along the lines of e.g. temporarily setting "y" or "n" in a cmd and then going back to the auto determined option afterwards, but you probably either know exactly what you want to go back to or should save a copy of the variable if you really care about the previous settings. So i'll go for integers 0=n,1=y with initial value determined at program startup.

As for variable name I think streamDebugColored rather than streamDebugColor in case somebody thinks there is the option to set the specific colour with the command.

dirk-zimoch commented 3 years ago

:+1:

FreddieAkeroyd commented 3 years ago

So I believe that is working, in that I can toggle the variable and see the control characters appear. Unfortunately something is up with my computer and I have to run "legacy consoles" which don't support the escapes so I can't tell if they would color the terminal, but if the coloring works for you on linux that is probably good enough. I'll see if i can get a colleague to build the branch and try it.

dirk-zimoch commented 3 years ago

Some compiler errors on Linux...

dirk-zimoch commented 3 years ago

With the above fix, it works on Linux. I can switch color on and off at run time. Also starting the ioc with stdout redirected to a file correctly initializes streamDebugColored off.