-
Notifications
You must be signed in to change notification settings - Fork 639
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
Implement ISpanAppendable in CharBlockArray and CharTermAttributeImpl #1028
Conversation
@NightOwl888 I intend to mark this ready for review once #1018 is merged, leaving as draft for now. I'll also likely have to resolve a merge conflict at that point too. |
…CharTermAttributeImpl
…rTermAttributeImpl.Append
bbd9f76
to
50b6702
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
There were a few more changes included than required. Copying spans is much less optimized on .NET Framework than using other copy methods, so we should add the Append(ReadOnlySpan<char>)
method without changing the implementations of the other overloads. Please see the inline comments.
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs
Outdated
Show resolved
Hide resolved
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs
Outdated
Show resolved
Hide resolved
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs
Outdated
Show resolved
Hide resolved
src/Lucene.Net.Tests/Analysis/TokenAttributes/TestCharTermAttributeImpl.cs
Show resolved
Hide resolved
All requested changes implemented
Implement
ISpanAppendable
inCharBlockArray
andCharTermAttributeImpl
Fixes #1026
Description
This adds
ISpanAppendable
to theCharBlockArray
andCharTermAttributeImpl
classes. The classes implement the interface explicitly so that we can provide a default implementation that returns itself rather thanISpanAppendable
, to match the existing Append overloads.Note that while
IAppendable
is now redundant in the interface list for these types, I left it there for Lucene source compatibility and reference, sinceISpanAppendable
does not exist in Lucene.