Replace boxing IList in HighlightChangedEventArgs and seal derived classes #9559
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM, @edwardneal can you resolve the above merge-conflicts ? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9559 +/- ##
===================================================
+ Coverage 13.43262% 13.58559% +0.15296%
===================================================
Files 3316 3316
Lines 664822 664822
Branches 74651 74651
===================================================
+ Hits 89303 90320 +1017
+ Misses 572923 571903 -1020
- Partials 2596 2599 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Thanks @dipeshmsft, I've just resolved them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The
Rangesproperty on the internal classHighlightChangedEventArgswas declared as anIList, but was always backed by an instance ofReadOnlyList<TextSegment>. Retrieving a list of text segments would thus always box an instance of theTextSegmentstruct. I've changed this property to declare it as anIList<TextSegment>.I also swapped an
ArrayListwith aList<int>in FixedDocument.cs, which stored a list of page numbers.Finally, I marked all the five classes derived from
HighlightChangedEventArgsas sealed. All fives of these classes were private, so there's no API surface change here.Customer Impact
Reduced memory usage/GC pressure.
Regression
No.
Testing
Local build and tests, sample application runs.
Risk
Low. The
Rangesproperty is only read byDocumentSequenceTextContainer,FixedDocumentandFlowDocument. In all three locations, it's indexed into and the result was explicitly cast to aTextSegment.Microsoft Reviewers: Open in CodeFlow