Skip to content

Fix hyperlinks being split into multiple links when text is highlighted#4110

Open
filbranden wants to merge 4 commits intoTextualize:masterfrom
filbranden:linkhighlight1
Open

Fix hyperlinks being split into multiple links when text is highlighted#4110
filbranden wants to merge 4 commits intoTextualize:masterfrom
filbranden:linkhighlight1

Conversation

@filbranden
Copy link
Copy Markdown

@filbranden filbranden commented May 1, 2026

Type of changes

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

AI?

  • AI was used to generate this PR

I used Claude Sonnet 4.6 to work on this fix, it was fully reviewed by a human though. The process went roughly as follows:

  • Claude initially suggested just dropping the copy() call, I told it there was probably a reason why it was there, so I asked it to figure that out and add a test that would ensure that behavior was preserved.
  • Claude then suggested a lighter approach to creating the copied style, which ended in an uncached refactor of _add that would get called in that specific situation. This looked acceptable. I asked Claude to add appropriate tests.
  • Then I realized if the issue was the lru_cache, we could fix it by passing the expected link_id as an argument, which would turn into a key for the lru_cache. In a sense, this seems more efficient because we don't need to copy or recreate or discard a style object. On the other hand, it might have side effects on the lru_cache itself, so I count on code review to confirm this is an appropriate approach.
  • I also asked Claude to add a specific test involving the highlighter, since that's the specific issue I was trying to fix.

I understand there wasn't discussion in the issue itself, I hope this PR with the code might serve as a starting point for discussion with a proposed code change that I'm willing to iterate on. Thanks!

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate (see note about typos above).
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review. YES PLEASE!

Description

Style.__add__ was calling copy() on every combined style that contained a
link, which generated a fresh link_id for each highlighted sub-span within
a single link markup region. Terminals use link_id to identify hyperlink
boundaries, so each sub-span appeared as a separate clickable link instead
of one continuous one.

The underlying root cause is that _add's lru_cache keys on __hash__/__eq__,
which intentionally excludes link_id. Two Style objects with the same visual
content but different link_ids therefore collide in the cache; copy() was
a blunt workaround that overcorrected by regenerating the id on every hit.

Fix this by having _add take the link_id as an argument so that the lru_cache
will now also key by link_id, preventing the issue that required the copy()
workaround altogether.

Add test cases to confirm the issue is fixed and include regression tests to prevent it from getting broken again in the future.

Fixes #4109.

filbranden added 4 commits May 1, 2026 08:31
…k styles

Two Style(link=...) objects with the same URL are equal by hash and comparison,
but carry different _link_id values. Combining each with a base style must
yield combined styles with distinct _link_ids; the lru_cache must not cause the
second result to inherit the first's _link_id.
Style.__add__ was calling copy() on every combined style that contained a
link, which generated a fresh link_id for each highlighted sub-span within
a single link markup region. Terminals use link_id to identify hyperlink
boundaries, so each sub-span appeared as a separate clickable link instead
of one continuous one.

The underlying root cause is that _add's lru_cache keys on __hash__/__eq__,
which intentionally excludes link_id. Two Style objects with the same visual
content but different link_ids therefore collide in the cache; copy() was
a blunt workaround that overcorrected by regenerating the id on every hit.

Fix this by having _add take the link_id as an argument so that the lru_cache
will now also key by link_id, preventing the issue that required the copy()
workaround altogether.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Hyperlinks are split into multiple links when text is highlighted

1 participant