marado / TalkerNode

A telnet-based talker, written in Node.js
The Unlicense
11 stars 11 forks source link

colors working incorrectly on tf #116

Open 3151 opened 4 years ago

3151 commented 4 years ago

While using the Talker with tf5 within tmux the colors changes for unknown reason. Its lines over lines in for example green, than it changes to blue for a lot of lines than blue. I could not see any trigger to that.

marado commented 4 years ago

We got more than one tf5 user with the same symptom, in the same running instance, where a telnet user sees no problem, so... maybe this is something reproducible with the use of tf5. Also, "after doing an .who everything is green, so it is as if the color reset command never came in."

marado commented 4 years ago
Mike says: I can see the problem in ncat
Mike says: there is no reset
Mike says: there is a [39m
Mike says: personally I'd throw a 0m in there
marado commented 4 years ago

This seems like a tf5 bug. Here's a command repro'ing it:

var formatters = require('../utils/formatters.js');

exports.command = {
    name: "tfcolors",
    autoload: true,
    unloadable: false,
    min_rank: 0,
    display: "debug command.",
    help: "debug command",
    usage: ".tfcolors",

    execute: function(socket, command, command_access) {

        var chalk = require('chalk');
        command_access.sendData(socket, chalk.blue("+blue+") + "no color");
        command_access.sendData(socket, "\r\n");
    }
}

This works well on several clients but not on tf5: While what is sent to the user is:

.tfc
ÿü^A^[[34m+blue+^[[39mno color^M

tf seems to completely ignore the "grey" command, and shows it all as blue.

marado commented 4 years ago

tf indeed does not support 39m:

/set start_color_black                  ^[[30m
/set start_color_red                    ^[[31m
/set start_color_green                  ^[[32m
/set start_color_yellow                 ^[[33m
/set start_color_blue                   ^[[34m
/set start_color_magenta                ^[[35m
/set start_color_cyan                   ^[[36m
/set start_color_white                  ^[[37m

/set start_color_bgblack                ^[[40m
/set start_color_bgred                  ^[[41m
/set start_color_bggreen                ^[[42m
/set start_color_bgyellow               ^[[43m
/set start_color_bgblue                 ^[[44m
/set start_color_bgmagenta              ^[[45m
/set start_color_bgcyan                 ^[[46m
/set start_color_bgwhite                ^[[47m
marado commented 4 years ago

According to the ANSI standard, 39 is "Default foreground color". While it is tf's fault that this isn't supported we should probably figure out a way to acomodate this (other than "disable colors").

Sending resets more often (eg,, after the output of each command) might be an interesting precaution, and mitigate (even if not totally fix) this issue.

I need more time to think, but... I might want to do two things here:

marado commented 4 years ago

Issue opened in tf5: https://sourceforge.net/p/tinyfugue/bugs-and-support/37/

marado commented 4 years ago

An after command reset will cause some problems, and would not fix all the potentially affected people (since we would only be able to send the chalk.reset() to the command caller). While I'm not very happy with the idea of implementing something like this, we might have to, and the solution will pass by reviewing everything that is socket-written, and spread reset()'s on the appropriate places.

We could - temporarily - hack all the writes and replace 39's by 0's (fg defaults with resets), but that would need to be fixed as soon as we tried to use background colors.

marado commented 4 years ago

Well... better yet, we can replace 39's by a default color (eg. white), that can actually be configurable in the talker. That would actually turn a "clients limitation" into a feature. Also, we should also deal with 49's (Default background color), which suffer from exactly the same problem.

marado commented 4 years ago

Unlike with the foreground color, there is a big difference between painting with a (talker defined) foreground color (instead of using the client-defined), and painting with a talker defined background color instead of using the clients: the background color might only affect the areas where text exists, and if you have a client with transparencies and so on, there's a big difference between having a defined background color and having none.

ie, we should probably find a way to "partially reset" colors... If we get a 39 (default foreground color), se can instead send a reset (0) + the previously defined background color (which we would need to know). And vice-versa. Doing this is a PITA unless it is done by chalk itself. Chalk has no way to do that now, and isn't properly "extensible" in order to do that. It might be feasible to alter chalk in order to support a "partial ANSI" mode that does not know 39 and 49 and deals with 0's instead... to investigate.

marado commented 3 years ago

Now that #87 is done, not only we have a reset in place, but it might even be triggering on the right places already. It would be useful to review this - how does TalkerNode with color on work with tf at the moment, and if there are still places where the color is bleeding, we can now easily deal with that.