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

Colors won't work for cmdrunner when using nodejs chalk or colorette library #618

Closed
abhayjatindoshi opened this issue Sep 7, 2019 · 8 comments · Fixed by #812
Closed
Labels
💪 improvement "Where we're going, we don't need roads."

Comments

@abhayjatindoshi
Copy link

What's broken?

Colors won't work for cmdrunner when using nodejs chalk or colorette library

@senorprogrammer
Copy link
Collaborator

See this issue for more details on why this is: #577

@abhayjatindoshi
Copy link
Author

I tried giving FORCE_COLOR=1 as env variable but it seems chalk and colorette both failed to give NC character after they had it coloured. It'll be great if this is supported from cmdrunner instead. As both the libraries detects no support for color but on the other hand if I check for iTerm.app it seems to detect 256-bit color support for my current configuration.

@senorprogrammer senorprogrammer added the 💪 improvement "Where we're going, we don't need roads." label Sep 7, 2019
@Seanstoppable
Copy link
Collaborator

@abhayjatindoshi Can you give more details on how you are using this? I.e. what is your config?

Have you tried wrapping this in a shell script and setting FORCE_COLOR=1 in that? CmdRunner probably doesn't use any environment variables passed to wtf

@abhayjatindoshi
Copy link
Author

I tried to run the node script wrapped in a shell file as below.

# ~/temp.sh
export FORCE_COLOR=1
node ~/folder-path/index.js arguments

When I execute temp.sh on iTerm.app it gives the expected output.
image

Even on Terminal.app it works well like below
image

But when I execute that file on wtfutil as I said before the NOCOLOR char doesn't work. so even the :: are getting coloured like below.
image

This is my configuration on config.yml for cmdrunner

    cmdrunner_web:
      type: 'cmdrunner'
      title: '⬆️ ⬇️'
      cmd: 'sh'
      args: ["/Users/mac-name/temp.sh"]
      enabled: true
      position: 
        top: 1
        left: 0
        height: 1
        width: 1
      refreshInterval: 5

@Seanstoppable

@Seanstoppable
Copy link
Collaborator

Seanstoppable commented Sep 12, 2019

Thanks for the additional info! That is very interesting. Can you run echo $TERM, both on a base terminal AND as part of the shell script? I imagine it is consistent and reasonable, but want to make sure I can mirror your env as exact as possible here.'

Also, can you share more of the program you are testing with?

@abhayjatindoshi
Copy link
Author

I've made up a small zip for you. It contains 3 files.

  1. main.sh - shell file that you can use in cmdrunner
  2. color.js - node js file which runs prints colored text using chalk library
  3. color.sh - shell fille that prints the colored text using echo

Setting up is simple. unzip to your home folder and run either sh ~/temp/main.sh file or cd ~/temp; npm install

When you run the main.sh you'll see that color is set also for "color" word in node chalk case when you run on cmdrunner. The same if executed in a shell it shows word "color" without any color.

@Seanstoppable

Download Zip code ⬇️

Shell output

image

CmdRunner output

image

@Seanstoppable
Copy link
Collaborator

I THINK this is a bug in tview and potentially in chaulk.

Note that the 2 scripts are not actually working the same. The shell script is clearing the style, and chaulk is setting the foreground color to the default (white)

Screen Shot 2019-09-13 at 7 59 12 PM

So chaulk is NOT clearing styles, so much as setting the default color. Tview doesn't handle default color, which I'll open a PR for.

@Seanstoppable
Copy link
Collaborator

PR opened at rivo/tview#347

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 improvement "Where we're going, we don't need roads."
Development

Successfully merging a pull request may close this issue.

3 participants