Textualize / rich

Rich is a Python library for rich text and beautiful formatting in the terminal.
https://rich.readthedocs.io/en/latest/
MIT License
49.71k stars 1.73k forks source link

[BUG] Text markup property opens new span tags before closing old ones. #3155

Open slaarti opened 1 year ago

slaarti commented 1 year ago

Describe the bug

When one Span ends and another Span starts at the same index of a Text, the output of the markup property will render the opening tag of the second before the closing tag of the first. I've actually written a unit test in preparation for possibly attempting to solve this myself and file a PR:

def test_markup_round_trip():
    src = "[red]foo[/red][blue]bar[/blue][yellow]baz[/yellow]"
    text = Text.from_markup(src)
    assert text.markup == src

And running the test:

=================================== FAILURES ===================================
____________________________ test_markup_round_trip ____________________________

    def test_markup_round_trip():
        src = "[red]foo[/red][blue]bar[/blue][yellow]baz[/yellow]"
        text = Text.from_markup(src)
>       assert text.markup == src
E       AssertionError: assert '[red]foo[blue][/red]bar[yellow][/blue]baz[/yellow]' == '[red]foo[/red][blue]bar[/blue][yellow]baz[/yellow]'
E         - [red]foo[/red][blue]bar[/blue][yellow]baz[/yellow]
E         + [red]foo[blue][/red]bar[yellow][/blue]baz[/yellow]

tests/test_text.py:103: AssertionError

I know there are probably corner cases where from_markup/markup round-tripping won't be 100% correct, but IMO it'd be nice to get closer.

(Also, I was tempted to compact the last two lines of the test into assert Text.from_markup(src).markup == src; would this be preferable, possibly also replacing src with text throughout for consistency with the other tests?)

Platform

Click to expand macOS Ventura, Python 3.11.6, iTerm2 3.4.21, Rich 13.6.0. ``` (rich-py3.11) $ python -m rich.diagnose ╭─────────────────────── ───────────────────────╮ │ A high level console interface. │ │ │ │ ╭──────────────────────────────────────────────────────────────────────────╮ │ │ │ │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ │ │ │ │ color_system = 'truecolor' │ │ encoding = 'utf-8' │ │ file = <_io.TextIOWrapper name='' mode='w' │ │ encoding='utf-8'> │ │ height = 26 │ │ is_alt_screen = False │ │ is_dumb_terminal = False │ │ is_interactive = True │ │ is_jupyter = False │ │ is_terminal = True │ │ legacy_windows = False │ │ no_color = False │ │ options = ConsoleOptions( │ │ size=ConsoleDimensions(width=80, height=26), │ │ legacy_windows=False, │ │ min_width=1, │ │ max_width=80, │ │ is_terminal=True, │ │ encoding='utf-8', │ │ max_height=26, │ │ justify=None, │ │ overflow=None, │ │ no_wrap=False, │ │ highlight=None, │ │ markup=None, │ │ height=None │ │ ) │ │ quiet = False │ │ record = False │ │ safe_box = True │ │ size = ConsoleDimensions(width=80, height=26) │ │ soft_wrap = False │ │ stderr = False │ │ style = None │ │ tab_size = 8 │ │ width = 80 │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─── ────╮ │ Windows features available. │ │ │ │ ╭───────────────────────────────────────────────────╮ │ │ │ WindowsConsoleFeatures(vt=False, truecolor=False) │ │ │ ╰───────────────────────────────────────────────────╯ │ │ │ │ truecolor = False │ │ vt = False │ ╰───────────────────────────────────────────────────────╯ ╭────── Environment Variables ───────╮ │ { │ │ 'TERM': 'xterm', │ │ 'COLORTERM': 'truecolor', │ │ 'CLICOLOR': None, │ │ 'NO_COLOR': None, │ │ 'TERM_PROGRAM': 'iTerm.app', │ │ 'COLUMNS': None, │ │ 'LINES': None, │ │ 'JUPYTER_COLUMNS': None, │ │ 'JUPYTER_LINES': None, │ │ 'JPY_PARENT_PID': None, │ │ 'VSCODE_VERBOSE_LOGGING': None │ │ } │ ╰────────────────────────────────────╯ platform="Darwin" (rich-py3.11) $ pip freeze | grep rich -e git+ssh://git@github.com/slaarti/rich.git@e9f75c9912ed25b9777bc0257853370951220b17#egg=rich (rich-py3.11) $ grep "^version" pyproject.toml version = "13.6.0" ```
github-actions[bot] commented 1 year ago

We found the following entry in the FAQ which you may find helpful:

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

willmcgugan commented 1 year ago

As long as the markup renders the expected output, then I don't consider it to be a problem.

Screenshot 2023-10-17 at 15 18 00

That said, I would accept a PR that more closely reflected the original markup

slaarti commented 1 year ago

Yeah, that's totally fair. I've put in PR #3163.