Skip to content

Character-only mutations should not overwrite paragraph-bound attributes in paragraph following mutation #1302

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jmschonfeld
Copy link
Contributor

When mutating text in an AttributedString that contains paragraph-bound attributes we fix-up the attributes post-mutation by looking at the paragraphs surrounding the mutation. There are up to two applicable paragraphs: one that begins before the mutation and ends somewhere within the mutation (the "starting" paragraph), and one that begins within the mutation and ends after the mutation (the "ending" paragraph). These may be the same paragraph (when the mutation is in the middle of the paragraph), or one or the other may not exist (if the mutation is at the start or the end of a paragraph).

Today, when the mutation is a character-based mutation (i.e. not just setting an attribute value) we always take the attributes from the beginning of the starting paragraph and apply them to the portion that extends into the mutation, and apply the attributes from the beginning of the ending paragraph (within the mutation) and apply them to the rest of the ending paragraph outside the mutation. This is correct for AttributedString based replacements (inserting an AttributedString into another AttributedString) but this isn't quite right for character based replacements (via the character view) where no attributes are supplied. Instead of the inferred attributes of the newly inserted text overwriting the ending paragraph, we should use the attributes from the existing ending paragraph in the inserted/mutated text. This mainly impacts when characters are inserted or mutated at the start of a paragraph and ensures it has the same behavior as when text is inserted anywhere else in the paragraph.

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test

@@ -257,7 +261,7 @@ class TestAttributedStringConstrainingBehavior: XCTestCase {

result = str
result.characters.replaceSubrange(result.index(result.startIndex, offsetByCharacters: 8) ..< result.index(result.startIndex, offsetByCharacters: 15), with: "Test\nReplacement")
verify(string: result, matches: [("Hello, wTest\n", 1), ("Replacementxt Paragraph", 1)], for: \.testParagraphConstrained)
verify(string: result, matches: [("Hello, wTest\n", 1), ("Replacementxt Paragraph", 2)], for: \.testParagraphConstrained)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an intentional change in behavior that I feel was previously incorrect. When replacing text across a paragraph boundary, we shouldn't overwrite the second paragraph with the first paragraph's attribute value, instead the two paragraphs should maintain their individual attribute values. This is unlike replacing the subrange with an AttributedString where the client supplies attributes that should extend from the replacement into the rest of the ending paragraph

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.

1 participant