rich icon indicating copy to clipboard operation
rich copied to clipboard

Issues with link IDs and style caching

Open rodrigogiraoserrao opened this issue 1 year ago • 2 comments

This is an account of many small related issues (not necessarily bugs, per se) uncovered while working on https://github.com/Textualize/textual/issues/1587

  • Hashing doesn't take into account the link ID, so styles that look the same but have different link IDs will clash.

  • Style._link_id is created inconsistently: the attribute _link_id is created in a couple of different places and is created inconsistently across those locations.

Some examples

https://github.com/Textualize/rich/blob/6d30ad0f30028210124c149811cbbe2b183711f9/rich/style.py#L192-L194

https://github.com/Textualize/rich/blob/6d30ad0f30028210124c149811cbbe2b183711f9/rich/style.py#L243

https://github.com/Textualize/rich/blob/6d30ad0f30028210124c149811cbbe2b183711f9/rich/style.py#L688

  • Style addition is cached in Style._add and the dunder method __add__ only refreshes the link (by using .copy on the result) if the result has a .link, but it doesn't check for .meta.

  • The method Text.render adds another layer of caching that bypasses the link refresh because of hashing clashes (maybe using the style indices instead of the styles themselves as cache keys fixes this).

I couldn't make much progress in solving these issues because I don't quite understand the semantics of the attribute .link_id. Is it supposed to be a different ID for any Style instance that contains a link or some meta data? Then, why not use the object ID itself?

@property
def link_id(self):
    return str(id(self)) if self.link or self.meta else ""

rodrigogiraoserrao avatar May 02 '23 10:05 rodrigogiraoserrao

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

github-actions[bot] avatar May 02 '23 10:05 github-actions[bot]

The _link_id is a unique id used when generating terminal links. It's sole purpose is to let the terminal know what to highlight when you hover over it.

willmcgugan avatar Sep 25 '23 09:09 willmcgugan