From 2c31fe99e1cc5bfdb393d4f5c70231a042ea67ef Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Mon, 11 Nov 2024 07:35:20 -0800 Subject: [PATCH] Use rval for AttributedString::Fragment changes (#47494) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47494 Changelog: [General][Changed] AttributedString `appendFragment` and `prependFragment` take an rval instead of a const ref; append/prependAttributedString have been removed Reviewed By: mdvacca Differential Revision: D65603864 fbshipit-source-id: 1160a9e2064470f826bea66736b4fce13caa3a73 --- .../attributedstring/AttributedString.cpp | 36 ++++--------------- .../attributedstring/AttributedString.h | 11 ++---- .../tests/AttributedStringBoxTest.cpp | 6 ++-- .../components/text/BaseTextShadowNode.cpp | 4 +-- .../AndroidTextInputShadowNode.cpp | 4 +-- 5 files changed, 15 insertions(+), 46 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.cpp b/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.cpp index 6ce203b415c..f5c0a1c020d 100644 --- a/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.cpp +++ b/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.cpp @@ -53,42 +53,18 @@ bool Fragment::operator!=(const Fragment& rhs) const { #pragma mark - AttributedString -void AttributedString::appendFragment(const Fragment& fragment) { +void AttributedString::appendFragment(Fragment&& fragment) { ensureUnsealed(); - - if (fragment.string.empty()) { - return; + if (!fragment.string.empty()) { + fragments_.push_back(std::move(fragment)); } - - fragments_.push_back(fragment); } -void AttributedString::prependFragment(const Fragment& fragment) { +void AttributedString::prependFragment(Fragment&& fragment) { ensureUnsealed(); - - if (fragment.string.empty()) { - return; + if (!fragment.string.empty()) { + fragments_.insert(fragments_.begin(), std::move(fragment)); } - - fragments_.insert(fragments_.begin(), fragment); -} - -void AttributedString::appendAttributedString( - const AttributedString& attributedString) { - ensureUnsealed(); - fragments_.insert( - fragments_.end(), - attributedString.fragments_.begin(), - attributedString.fragments_.end()); -} - -void AttributedString::prependAttributedString( - const AttributedString& attributedString) { - ensureUnsealed(); - fragments_.insert( - fragments_.begin(), - attributedString.fragments_.begin(), - attributedString.fragments_.end()); } void AttributedString::setBaseTextAttributes( diff --git a/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.h b/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.h index 4499aefad45..7bd57b37c13 100644 --- a/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.h +++ b/packages/react-native/ReactCommon/react/renderer/attributedstring/AttributedString.h @@ -65,15 +65,8 @@ class AttributedString : public Sealable, public DebugStringConvertible { /* * Appends and prepends a `fragment` to the string. */ - void appendFragment(const Fragment& fragment); - void prependFragment(const Fragment& fragment); - - /* - * Appends and prepends an `attributedString` (all its fragments) to - * the string. - */ - void appendAttributedString(const AttributedString& attributedString); - void prependAttributedString(const AttributedString& attributedString); + void appendFragment(Fragment&& fragment); + void prependFragment(Fragment&& fragment); /* * Sets attributes which would apply to hypothetical text not included in the diff --git a/packages/react-native/ReactCommon/react/renderer/attributedstring/tests/AttributedStringBoxTest.cpp b/packages/react-native/ReactCommon/react/renderer/attributedstring/tests/AttributedStringBoxTest.cpp index b9ddd65ef86..b6774b36f4d 100644 --- a/packages/react-native/ReactCommon/react/renderer/attributedstring/tests/AttributedStringBoxTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/attributedstring/tests/AttributedStringBoxTest.cpp @@ -23,7 +23,7 @@ TEST(AttributedStringBoxTest, testValueConstructor) { auto attributedString = AttributedString{}; auto fragment = AttributedString::Fragment{}; fragment.string = "test string"; - attributedString.appendFragment(fragment); + attributedString.appendFragment(std::move(fragment)); auto attributedStringBox = AttributedStringBox{attributedString}; EXPECT_EQ(attributedStringBox.getMode(), AttributedStringBox::Mode::Value); @@ -58,7 +58,7 @@ TEST(AttributedStringBoxTest, testMoveConstructor) { auto attributedString = AttributedString{}; auto fragment = AttributedString::Fragment{}; fragment.string = "test string"; - attributedString.appendFragment(fragment); + attributedString.appendFragment(std::move(fragment)); auto movedFromAttributedStringBox = AttributedStringBox{attributedString}; auto moveToAttributedStringBox = @@ -88,7 +88,7 @@ TEST(AttributedStringBoxTest, testMoveAssignment) { auto attributedString = AttributedString{}; auto fragment = AttributedString::Fragment{}; fragment.string = "test string"; - attributedString.appendFragment(fragment); + attributedString.appendFragment(std::move(fragment)); auto movedFromAttributedStringBox = AttributedStringBox{attributedString}; auto moveToAttributedStringBox = AttributedStringBox{}; diff --git a/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.cpp index 77ede813aed..1d0bd67846c 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.cpp @@ -48,7 +48,7 @@ void BaseTextShadowNode::buildAttributedString( // don't need it at all). Storing a `ShadowView` instance instead of // `ShadowNode` should properly fix this problem. fragment.parentShadowView = shadowViewFromShadowNode(parentNode); - outAttributedString.appendFragment(fragment); + outAttributedString.appendFragment(std::move(fragment)); lastFragmentWasRawText = true; } continue; @@ -75,7 +75,7 @@ void BaseTextShadowNode::buildAttributedString( fragment.string = AttributedString::Fragment::AttachmentCharacter(); fragment.parentShadowView = shadowViewFromShadowNode(*childNode); fragment.textAttributes = baseTextAttributes; - outAttributedString.appendFragment(fragment); + outAttributedString.appendFragment(std::move(fragment)); outAttachments.push_back(Attachment{ childNode.get(), outAttributedString.getFragments().size() - 1}); } diff --git a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp index 2d19e6daecf..e45bca14842 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp @@ -61,7 +61,7 @@ AttributedString AndroidTextInputShadowNode::getAttributedString() const { // that effect. fragment.textAttributes.backgroundColor = clearColor(); fragment.parentShadowView = ShadowView(*this); - attributedString.prependFragment(fragment); + attributedString.prependFragment(std::move(fragment)); } return attributedString; @@ -91,7 +91,7 @@ AttributedString AndroidTextInputShadowNode::getPlaceholderAttributedString() // appended to the AttributedString (see implementation of appendFragment) fragment.textAttributes = textAttributes; fragment.parentShadowView = ShadowView(*this); - textAttributedString.appendFragment(fragment); + textAttributedString.appendFragment(std::move(fragment)); return textAttributedString; }