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

Colored text output renders differently in Screen than in regular terminal #326

Closed
bkjohnson opened this issue Sep 6, 2021 · 9 comments
Closed
Assignees

Comments

@bkjohnson
Copy link

Describe the bug

When using screen.paint with ColouredText and AsciimaticsParser, characters are not always rendered properly. Sometimes the spacing is messed up, other times some of the encoding characters are printed, and other times the text seems to be getting truncated.

To Reproduce

Here is a sample that should show the whitespace and truncating problems. See Additional context section.

from asciimatics.screen import Screen
from asciimatics.parsers import AsciimaticsParser
from asciimatics.strings import ColouredText


def render_code_block(screen, block, x, row):
    cur_row = row
    lines = block.split("\n")

    for line in lines:
        text = ColouredText(
            line,
            AsciimaticsParser(),
        )
        screen.paint(
            text.raw_text,
            x,
            cur_row,
            colour_map=text.colour_map,
        )
        cur_row += 1


def demo(screen):
    line = '\x1b[38;5;28mprint\x1b[39m(\x1b[38;5;124m"\x1b[39m\x1b[38;5;124mThis is a test\x1b[39m\x1b[38;5;124m"\x1b[39m)'

    line2 = "console.log(\x1b[38;5;124m`\x1b[39m\x1b[38;5;124mData is \x1b[39m\x1b[38;5;132;01m${\x1b[39;00mmyFunc(\x1b[38;5;124m'John'\x1b[39m)[\x1b[38;5;241m0\x1b[39m]\x1b[38;5;132;01m}\x1b[39;00m\x1b[38;5;124m`\x1b[39m);"

    while True:
        # rendering this line will show spacing problems
        render_code_block(screen, line, 5, 10)

        # rendering this line will show spacing issues and the truncating issue
        render_code_block(screen, line2, 5, 15)

        ev = screen.get_key()
        if ev in (ord("Q"), ord("q")):
            return
        screen.refresh()


Screen.wrapper(demo)

Expected behavior
I expect the text to be painted to the screen like it is printed in the terminal.

Screenshots

Here is a screenshot of what the above sample code gives:

image

Here's what I get from a repl when I print the same encoded strings that I'm passing to ColouredText:

image

If I change the second call to render_code_block to have a 0 instead of a 5 for the x argument, I get to see part of "John":

image

System details (please complete the following information):

  • OS and version: Linux Mint 20.1
  • Python version: 3.8.10
  • Python distribution: Default from apt repo for Ubuntu
  • Asciimatics version 1.13.0

Additional context

I came across this while trying to add a feature to the present tool. asciimatics is used heavily in that project, and I haven't come up with a minimum example that shows the encoding characters rendered. I suspect this may have to do with Scenes, Effects, etc. being used. Here's an example of what that looks like when it happens:

image

@peterbrittain
Copy link
Owner

I suspect you are using the wrong parser. Looking at your code, you are passing Ansi escape codes into the string, but then using the Asciimatics parser to decode them. This parser just looks for ${c,a,b} escape sequences (as covered in https://asciimatics.readthedocs.io/en/stable/widgets.html#changing-colours-inline).

Try using the AnsiTerminalParser (https://asciimatics.readthedocs.io/en/stable/asciimatics.html#asciimatics.parsers.AnsiTerminalParser) instead.

@peterbrittain peterbrittain self-assigned this Sep 6, 2021
@peterbrittain
Copy link
Owner

Actually... There are 2 other bugs here. First is that you should never use the raw_text property. You should just use your ColouredText like a string (that happens to have an extra colour_map property) - i.e. remove ".raw_text" from your code.

The last is that this sample assumes that escape code 39 maps to using white foreground text. The AnsiTerminalParser doesn't support this mapping, but it is an easy fix if you need it. LMK if you need the mapping...

@bkjohnson
Copy link
Author

I didn't notice a difference when I removed .raw_text, but I've switched to that anyway. However, using AnsiTerminalParser solved my problem! I tried using this initially when exploring options and then ended up getting something that looked close enough with AsciimaticsParser, so I went with that. The earlier problems may have been related to not using pygments correctly.

Also, since we can pass in a full ColouredText object to paint, how do you feel about changing the method to use the colour map from the ColouredText object if the colour_map argument is None?

I don't totally understand the details of the escape code 39 issue, but is it causing the parenthesis after string arguments to still be red? If so, adding that fix would be fantastic.

image

@peterbrittain
Copy link
Owner

Done. Your brackets should now work with the latest code from 1fde70e.

@peterbrittain
Copy link
Owner

Oh and I'm a little nervous about just looking for the colour map on ColouredText inside paint... While a handy shortcut for this case, I worry that the implicit API might have unintended consequences elsewhere.

@bkjohnson
Copy link
Author

Thank you for the fix! I tested it locally by installing based on the hash and it's working as expected. Will there be a pip release?

No worries about the implicit colour_map. I ended up changing my entire approach and using a custom renderer that overrides the rendered_text property, so I'm not even calling paint directly anymore.

@peterbrittain
Copy link
Owner

I don't have a fixed release cycle for asciimatics. I just let content accrue and then release when it reaches critical mass. I wouldn't normally expect that for several more months yet... But LMK if the present package needs to push something out sooner and I'll see what I can do.

BTW, if you're looking at renderers for integration, you might want to consider the new AsciinemaPlayer as a way to easily record and replay existing coloured terminal content. If you don't need the animation, there is a very simple API to the underlying AbstractScreenPlayer to allow you to fix up the whole page in one shot instead.

@peterbrittain
Copy link
Owner

I assume that this is now addressed and so am closing. Feel free to reach out if you need the new release.

@bkjohnson
Copy link
Author

Yes, it's been addressed. Thank you for the help.

I may take a look at the AsciinemaPlayer if my current PR goes well, thanks for the tip.

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

No branches or pull requests

2 participants