Hovering links in markup changes state of all links with identical action / parameter

[mitosch]

Issue Description

Hovering one link will change the hover state of all links having the same action/params.

Given the example below, the links 1, 3 and 4 are changing the state together:

Further analysis:

• The link_id of those 3 links are the same (see textual log).

Working Example

from __future__ import annotations

from textual.app import App, ComposeResult
from textual.widgets import Header, Footer, Static

from textual import log


TEXT = """
Lorem ipsum dolor sit [@click=link('amet')](1) amet[/]
 , consetetur sadipscing elitr, sed diam nonumy
eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam
voluptua. At [@click=link('vero')](2) vero[/] eos et accusam et justo duo
dolores et ea rebum.

Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor
sit amet. Lorem ipsum dolor sit [@click=link('amet')](3) amet[/], consetetur
sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore
magna aliquyam erat, sed diam voluptua. At [@click=link('amet')](4) vero[/]
eos et accusam et justo duo dolores et ea rebum.

Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum
dolor sit [@click=link('no-amet')](5) amet[/].
"""


class LinkableStatic(Static):
    def watch_highlight_link_id(self, link_id: str) -> None:
        log("highlight link:", link_id)


class LinkTesterApp(App):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Footer()

        yield LinkableStatic(TEXT)


if __name__ == "__main__":
    app = LinkTesterApp()
    app.run()

[github-actions]

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

[rodrigogiraoserrao]

Hey @mitosch, just want to let you know that I'm working on a fix for this and I have narrowed down the cause of the issue after banging my head against the wall for a moderate amount of time.
Expect a fix in the upcoming days. 🚀

[rodrigogiraoserrao]

@willmcgugan I'll probably block this issue until there's some clarity on a path forward via resolution of Textualize/rich#2949.

Or do you think I should try to work around rich#2949 to fix this symptom in Textual?

[willmcgugan]

I don't think this is a Rich issue per se, but is clearly related to link_id. Please investigate more and add details to this issue.

[rodrigogiraoserrao]

So far, I've been able to determine that disabling the caches in Text.render and Style._add at the same time fixes this issue.
However, disabling just one or the other isn't enough to fix the issue. 🤔

[rodrigogiraoserrao]

Opened a draft PR with a solution.
I'm not 100% sure that's THE solution.

The reasoning and my findings are included in the PR and also included verbatim under the toggle below.

Reasoning & findings

The issue we're dealing with boils down to the fact that the attribute Style._link_id is not taken into account when hashing a Style which then introduces caching collisions with styles that have the same _link/_meta attributes but that have a different _link_id.

Can't we include _link_id in the hash of a style?

We can't, because we don't want equality comparison to depend on _link_id, and if two objects are compared as equal, then their hash should be the same.

When working on #1587, the first fix I found was that I could disable the cache inside rich.text.Text.render and the functools.lru_cache around rich.style.Style._add and that would fix the issue.
So, this does show it is a caching issue.
In particular, it has to do with the clashes that can exist when styles have the same meta values but originally have different _link_id attributes:

from rich.style import Style
meta = {"click": "something"}
s1 = Style.from_meta(meta)
s2 = Style.from_meta(meta)
print(s1.link_id, s2.link_id)  # Different link_id.
# (
#     '6311506187629892456089155',
#     '4273756187629892456089155'
# )

base = Style(color="red")
print((base + s1).link_id)  # This is the link_id of s1.
# '6311506187629892456089155'

# Now, we add base to s2:
#             vv
print((base + s2).link_id)  # This is the link_id of s1!
# '6311506187629892456089155'

Now, we can take a look at how addition is implemented and this reveals a promising workaround:

class Style:
    # ...
    def __add__(self, style: Optional["Style"]) -> "Style":
        combined_style = self._add(style)
        return combined_style.copy() if combined_style.link else combined_style

The call to .copy() refreshes the link_id if the resulting combined style has a link.
So, one might think it is a good idea to change the return statement to

return combined_style.copy() if combined_style.link or combined_style.meta else combined_style

However, this breaks link highlighting on the Textual side entirely – as in, no link gets any highlighting – because Textual relies on the link IDs to know what to highlight.
When the markup is first parsed, Textual will keep tabs on the link IDs of the styles it created, so if the rendering of the widget will come back with a style with a new link ID, Textual won't know what to highlight.

A potential fix is to bypass the cache entirely when adding styles that have link IDs but no links.