Skip to content

AttributedString character insertion doesn't invalidate text dependent attributes #1256

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.

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

Merged

Conversation

jmschonfeld
Copy link
Contributor

@jmschonfeld jmschonfeld commented Apr 21, 2025

When inserting a character into an attribute run, if that run contains any text-dependent attributes we should invalidate those attributes. However if the text is inserted between two runs of that attribute value, neither surrounding run should be invalidated. This updates enforceAttributeConstraintsBeforeMutation(to:) to ensure that we handle an empty range mutation (i.e. an insertion) correctly rather than skipping constraint enforcement for empty ranges.

rdar://149623441

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test


// Inserting text between two runs should not invalidate text dependent attributes in either of the surrounding runs
str.characters.insert("|", at: str.index(str.startIndex, offsetByCharacters: 5))
verify(string: str, matches: [("Hello", 1, 2), ("|", 1, nil), ("World", 1, 3)], for: \.testInt, \.testCharacterDependent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the previous behavior[("Hello", 1, nil), ("|", 1, nil), ("World", 1, nil)]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous behavior here would have been [("Hello", 1, 2), ("|", 1, nil), ("World", 1, 3)] as well, because we previously ignored invalidation for all insertions. But now that we're handling insertions and invalidating them in other places, I just added this unit test to ensure this doesn't break the invariant that invalidations shouldn't happen when inserting between runs. The test that previously failed without this change is the one above where inserting "A" into "Hello" would result in the surrounding "Hello" still having the text dependent attribute value set.

@jmschonfeld jmschonfeld merged commit 655dd5e into swiftlang:main Apr 21, 2025
3 checks passed
@jmschonfeld jmschonfeld deleted the attrstr/text-dependent-insertion branch April 21, 2025 19:05
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.

2 participants