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

Pass colorized output to log from underlying command #108

Closed
nsbruce opened this issue Dec 13, 2023 · 10 comments · Fixed by #111
Closed

Pass colorized output to log from underlying command #108

nsbruce opened this issue Dec 13, 2023 · 10 comments · Fixed by #111

Comments

@nsbruce
Copy link

nsbruce commented Dec 13, 2023

If the task being run has a colorized output when run without xc, I think xc should pass that colorized output through.

Trivial example:

20231213_09h58m08s_grim

A more valid example is that when using testing suites like pytest where successful tests are shown in green and failures in red.

@joerdav
Copy link
Owner

joerdav commented Dec 20, 2023

@stephenafamo I think this may be related to the log writer you implemented. Let me know if you have time to look at this :)

@stephenafamo
Copy link
Contributor

image

Colorized output works. I suspect the issue is somehow in checking if the terminal supports colors, and this is likely to be more of an issue with the shell interpreter than with the log writer.

To prevent using the log writer, try adding interactive: true to the task definition and see what it outputs @nsbruce

@stephenafamo
Copy link
Contributor

I got some more time to look at this, and it is indeed the logwriter.

The issue is that when checking if output should be colourized, commands like ls check if the output stream is a terminal.
From my first look around, there is no way to fake this, so I am conflicted about how best to proceed.

The most likely solution is that these tools decide to "FORCE_COLOR".

@joerdav
Copy link
Owner

joerdav commented Jan 3, 2024

I'm also under the impression that we cannot fake a terminal in this case. What I'm thinking is that we could set CLICOLOR_FORCE=1 so long as NO_COLOR is not set.

@stephenafamo
Copy link
Contributor

I'm also under the impression that we cannot fake a terminal in this case. What I'm thinking is that we could set CLICOLOR_FORCE=1 so long as NO_COLOR is not set.

I think we should set both CLICOLOR_FORCE and FORCE_COLOR but first check if the caller is a terminal.

@joerdav
Copy link
Owner

joerdav commented Jan 3, 2024

Good point, will look into that.

@joerdav
Copy link
Owner

joerdav commented Jan 12, 2024

Created a fix branch for this, would love to see if it works for people before merging!

@nsbruce
Copy link
Author

nsbruce commented Jan 12, 2024

I just cloned main, switched to term-colors branch, ran go install ./cmd/xc and am getting the same result as when I posted the issue.

@joerdav
Copy link
Owner

joerdav commented Jan 15, 2024

That's unfortunate, is this still the ls command?

@nsbruce
Copy link
Author

nsbruce commented Jan 15, 2024

Hey I apologize but it is working. To make troubleshooting easier I just spun up a MWE to show you it not working but must have erred the other day. It is properly colorizing now.

from ubuntu:22.04                                                                                                            
                                                                                                                               
run apt-get update && apt-get -y install git curl                                                                            
                                                                                                                               
run rm -rf /usr/local/go && \                                                                                                
  curl -L -o go1.21.6.linux-386.tar.gz https://go.dev/dl/go1.21.6.linux-386.tar.gz && \                                
  tar -C /usr/local -xzf go1.21.6.linux-386.tar.gz                                                                     
                                                                                                                               
env PATH $PATH:/usr/local/go/bin                                                                                             
env GOBIN /usr/local/go/bin                                                                                                  
                                                                                                                               
run git clone https://github.com/joerdav/xc && \                                                                             
cd xc && \                                                                                                           
  git switch term-colours && \                                                                                         
   git pull && \                                                                                                        
   go install ./cmd/xc                                                                                                  
                                                                                                                               
run echo '## Tasks\n\n### ls\n\n```bash\nls --color\n```' > /README.md                                                       
                                                                                                                               
entrypoint cd / && ls --color && xc ls

Then running docker build -t xc-test . && docker run --rm -it xc-test

20240115_08h01m49s_grim

Thanks for the work on this! I look forward to the merge.

@nsbruce nsbruce closed this as completed Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants