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.

Sign up for GitHub

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

Jump to bottom

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

Open
2 tasks done
slaarti opened this issue Oct 16, 2023 · 3 comments · May be fixed by #3163
Open
2 tasks done

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

slaarti opened this issue Oct 16, 2023 · 3 comments · May be fixed by #3163
Labels
Needs triage

Comments

@slaarti
Copy link

slaarti commented Oct 16, 2023

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
╭─────────────────────── <class 'rich.console.Console'> ───────────────────────╮
│ A high level console interface.                                              │
│                                                                              │
│ ╭──────────────────────────────────────────────────────────────────────────╮ │
│ │ <console width=80 ColorSystem.TRUECOLOR>                                 │ │
│ ╰──────────────────────────────────────────────────────────────────────────╯ │
│                                                                              │
│     color_system = 'truecolor'                                               │
│         encoding = 'utf-8'                                                   │
│             file = <_io.TextIOWrapper name='<stdout>' 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                                                        │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─── <class 'rich._windows.WindowsConsoleFeatures'> ────╮
│ 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://[email protected]/slaarti/rich.git@e9f75c9912ed25b9777bc0257853370951220b17#egg=rich
(rich-py3.11) $ grep "^version" pyproject.toml
version = "13.6.0"
@github-actions
Copy link

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
Copy link
Collaborator

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 slaarti linked a pull request Oct 21, 2023 that will close this issue
Open
9 tasks
@slaarti
Copy link
Author

slaarti commented Oct 21, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Needs triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Swap open-vs-close tag order when converting Text to markup. slaarti/rich
2 participants
@willmcgugan @slaarti