Style link id cache

[rodrigogiraoserrao]

Type of changes

• Bug fix
• New feature
• Documentation / docstrings
• Tests
• Other

Checklist

• I've run the latest black with default args on new code.
• I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
• I've added tests for new code.
• I accept that @willmcgugan may be pedantic in the code review.

The changes proposed here pass all tests.
No regression tests have been added yet.

Description

This follows Textualize/textual#1587 and #2949.
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 Textualize/textual#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.

[codecov-commenter]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (aabfd16) 98.30% compared to head (7eb7c92) 98.28%.
Report is 15 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3136      +/-   ##
==========================================
- Coverage   98.30%   98.28%   -0.03%     
==========================================
  Files          74       74              
  Lines        8038     8049      +11     
==========================================
+ Hits         7902     7911       +9     
- Misses        136      138       +2     

Flag	Coverage Δ
unittests	98.28% <96.15%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
rich/console.py	98.04% <100.00%> (+<0.01%)	⬆️
rich/markdown.py	99.45% <100.00%> (ø)
rich/style.py	99.53% <100.00%> (+<0.01%)	⬆️
rich/syntax.py	98.19% <100.00%> (-0.29%)	⬇️
rich/text.py	99.01% <87.50%> (-0.17%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rodrigogiraoserrao]

@willmcgugan if I recall correctly, when I tackled this we talked a bit and concluded this wasn't trivial. This PR was a draft because it was a proposed solution.
I'll be happy to mark it as ready if you prefer that to review it.

[willmcgugan]

If you think it is a possible solution, please mark it as ready. We'll go over it at some point. Need to refresh my memory.

[willmcgugan]

I don't quite grok this enough to merge. Closing for now, will look in to it again when I tackle the original issue.