Feature - Implemented Terminal color output

O

Obeliks

Member
I have implemented color output on the command line, very nice when running scripts on remote terminals via SSH.

I've used the Jansi library that supports both Unix and Windows terminals, and I've successfully tested it even on Windows cmd prompt.

1617739142681.png1617738516676.png

It supports both named HTML colors and hex codes, and uses the nearest color on terminals that only support 16 or 256 colors. Auto-detection of features is done by Jansi, but can be overridden on the command line.

The code can be seen here: https://github.com/oxc/kolmafia/compare/upstream...color_cli
(UPDATE: I have attached both commits as patches as well)
(UPDATE: Patches can be downloaded here: https://github.com/oxc/kolmafia/compare/upstream...color_cli.patch )

You can test it yourself using the latest build here (built by GitHub Actions, in case you're concerned): https://github.com/oxc/kolmafia/releases/tag/latest

Comments and feedback welcome! Please let me know if changes are required to get this integrated into KoLmafia.
 
Last edited:
I just noticed that my code replicates some features that are already part of Jansi, so I will strip it down a bit tomorrow.

UPDATE: I have pushed the changes and updated the 2nd patch
 
Last edited:
Both patches install and compile on MacOSX big sur. I can add the Hello World statement from the docs to my KoLmafia.java and get colored output in Terminal.

Can you generate a few unit tests?

Does it fail gracefully on non-supported platforms? Are there any non-supported platforms? Do any of those Kernel32 functions allow developers or users to do disallowed or dubious things (e.g. write to files outside of the KoLmafia security chroot ...)?

Is it necessary to install the jni code for each platform? How much does this add to the build size? (not much)

Does it work when developers are looking at console output inside an IDE, including Eclipse, VSCode, and IntelliJ?
 
MCroft said:
Both patches install and compile on MacOSX big sur. I can add the Hello World statement from the docs to my KoLmafia.java and get colored output in Terminal.

Can you generate a few unit tests?
Click to expand...
This is a bit tricky. What would you like tested?

MCroft said:
Does it fail gracefully on non-supported platforms? Are there any non-supported platforms?
Click to expand...
As far as I understand the documentation and source code of the Jansi library, it will not cause and problems or inconvenience on any non-supported platform. I'm not sure how to test this though.

MCroft said:
Do any of those Kernel32 functions allow developers or users to do disallowed or dubious things (e.g. write to files outside of the KoLmafia security chroot ...)?
Click to expand...
I assume you mean ASH/js developers (Java developers that change the source can do anything anyway). I don't see how any of those functions would be exposed to the script runtime. Also, from the source code of the native libraries, the only questionable action I see is the native "chdir" method that allows changing the current working directory.

What exactly is the KoLmafia security chroot, and how is it enforced?


MCroft said:
Is it necessary to install the jni code for each platform? How much does this add to the build size? (not much)
Click to expand...
I have built the daily jar for 20675 with and without my changes:
20552111 bytes without vs.
20775180 bytes with

So ~200k. Seems reasonable to me.
MCroft said:
Does it work when developers are looking at console output inside an IDE, including Eclipse, VSCode, and IntelliJ?
Click to expand...
Works for me out of the box in IntelliJ IDEA, and I know there is a ANSI console plugin for Eclipse which I used to use. I'm most certain it also works in VSCode.
 
This is awesome. Though the regex needs some improvement. It could easily break if the user calls print_html() with a string containing multiple <font> tags or nested <font> tags.

Edge cases I can think of:

HTML: 
<font color="red">red text</font> default color <font color="blue">blue text</font>
<font color="red">red text <font color="blue">blue text</font> red text</font>

Perhaps it would be better to manually tokenize the incoming HTML, scanning for opening/closing <font> tags and emitting color codes appropriately.

P.S. You can generate the patch file directly from a GitHub URL:
 
philmasterplus said:
Perhaps it would be better to manually tokenize the incoming HTML, scanning for opening/closing <font> tags and emitting color codes appropriately.
Click to expand...
Yes, you are absolutely right. I would usually be the first to contest using regexp for HTML "parsing", but it seemed to fit the KoLmafia style :p

I will look into what the code base already uses for use-cases like this, and come up with a better solution.
 
What do you think should happen if the string contains other HTML tags than <font>? Should I skip the formatting completely in that case, or treat them as verbatim text and just replace the font tags?
 
In an ideal world, I would yearn for a dedicated HTML renderer for the CLI. Something that understands CSS, tables, and layout.

Practically? I think your solution is fine. Let's ignore <span> and style="...".
 
I have changed the font parsing to use HtmlCleaner (which is already included in the project). I've encapsulated some of the functionality in their own classes based on separate concerns. This makes it easer to write unit tests (which are now also included), and to extend the functionality in the future, for example to support tables and whatnot.

I have added unit tests for several edge cases including the ones you mentioned.

Here is a small example screenshot for some of them:
1617807624658.png

One important thing: the parser now removes all HTML tags that are not <font> from the CLI output, and only outputs their text content (this does not apply to mirror or debug streams). Is this acceptable?
 
Great! Could we have a per-user config flag to disable the CLI colors (and HTML processing) explicitly? If so, all would be golden for me :)
 
Last edited:
Obeliks said:
One important thing: the parser now removes all HTML tags that are not <font> from the CLI output, and only outputs their text content (this does not apply to mirror or debug streams). Is this acceptable?
Click to expand...

I was about to express a concern but then realized this is the CLI and not the gCLI, correct? I have a vague recollection of messing with the CLI and one of my developer only test cases was to send the same output to both places and verify that the difference in display output was exactly what I expected.
 
This would have been great a decade ago when I was playing KoL from a remote location over a shared geosynchronous satellite connection by ssh to my home computer where KoLmafia was running. I'm glad I'm not doing that any more.
 
Obeliks said:
One important thing: the parser now removes all HTML tags that are not <font> from the CLI output, and only outputs their text content (this does not apply to mirror or debug streams). Is this acceptable?
Click to expand...
I've decided that it makes more sense to have all other tags in the output, but printed in gray, otherwise stuff like tables looks really garbled. It still does not look like much, but better than having the unformatted HTML, and better than just the text:

1617911953019.png
 
Is there something I can approve on this patch to get it merged?
I have been running it in its current state for a while now without any problems.

Are there any open concerns I can address?
 
I've just tried this patch and it is beautiful. The increase in JAR size is ~150 KiB (~1% of r20685) which is of little concern to me.

I hope this gets integrated soon. Until then I'll be manually building color-enabled JARs just to enjoy the colors.
 
Post it again, here, and then PM me daily until I review and commit. It is not clear to me that the posted or GIT generated patch is the patch people were testing and having a file here gives an audit trail I am comfortable with.
 
You must log in or register to reply here.
Back
Top