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

Add pretty printing without colour #73

Closed
wants to merge 1 commit into from

Conversation

0rphee
Copy link

@0rphee 0rphee commented Dec 22, 2023

No description provided.

@0rphee 0rphee requested a review from vrom911 as a code owner December 22, 2023 20:06
@0rphee
Copy link
Author

0rphee commented Dec 22, 2023

Pinging just in case @tomjaguarpaw

@tomjaguarpaw
Copy link
Contributor

Thanks, I wasn't subscribed to the repo (but I am now).

This seems reasonable. Is it for your work including stan into HLS?

It's interesting that colourista seems to embed escape codes straight into the string. I was expecting it to use some structured format that we could erase colour from, but maybe not.

We should wait to see if @vrom911 has any objections. She's the project owner and I'm just an assistant. In the meantime can you say more about the use case?

@0rphee
Copy link
Author

0rphee commented Dec 22, 2023

This seems reasonable. Is it for your work including stan into HLS?

Precisely.

In the meantime can you say more about the use case?

For the hls plugin, initially I used plain show's 😅 for the logging, but I was requested to provide a more friendly output, so I tried using the pretty printer functions here, however, they leave the terminal escape codes in the log files, and thus, here's the PR.

Nevertheless, thinking about it now, (if approved), it may be better to delay a bit the merging of this until I get the green flag that I get to actually use them haha...

@tomjaguarpaw
Copy link
Contributor

OK, makes sense. Can I suggest that in the first instance you take the low-tech hacky approach of just stripping everything in the string that occurs between pairs of \ESC and m1? That will get you the same outcome, but avoid changing this library (which I'm hesitant to do at the moment).

Footnotes

  1. That's the format for terminal escape codes that deal with presentation

@0rphee
Copy link
Author

0rphee commented Jan 8, 2024

Seems fine :) I'll see what I can do

@tomjaguarpaw
Copy link
Contributor

Thanks. Keep me updated on how that goes and then we'll think about what's the best long-term solution.

@0rphee
Copy link
Author

0rphee commented Jan 9, 2024

Sure, thanks. Your solution works well.

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 this pull request may close these issues.

2 participants