Add phrase_limit to highlighting #2959
Conversation
- Change order from string to HighlighterOrder? - Change TagsSchema from string to HighlighterTagsSchema? - Remove BoundaryMaxSize - Change Encoder to HighlighterEncoder? - Remove encoder from highlight fields as it is only valid on the top level highlighter - Add Encoder and PhraseLimit to integration test Closes #2851
| @@ -215,13 +206,13 @@ public class HighlightDescriptor<T> : DescriptorBase<HighlightDescriptor<T> ,IHi | |||
| ); | |||
|
|
|||
| // <inheritdoc/> | |||
| public HighlightDescriptor<T> TagsSchema(string schema = "styled") => Assign(a => a.TagsSchema = schema); | |||
| public HighlightDescriptor<T> TagsSchema(HighlighterTagsSchema? schema) => Assign(a => a.TagsSchema = schema); | |||
There was a problem hiding this comment.
I can make it the default, but the client generally doesn't have default values for nullable enums. I suspect this had a default value more as a hint for the accepted string value.
|
|
||
| // <inheritdoc/> | ||
| public HighlightDescriptor<T> RequireFieldMatch(bool requireFieldMatch) => Assign(a => a.RequireFieldMatch = requireFieldMatch); | ||
| public HighlightDescriptor<T> RequireFieldMatch(bool requireFieldMatch = true) => Assign(a => a.RequireFieldMatch = requireFieldMatch); |
There was a problem hiding this comment.
New default of true changes behaviour - is this intentional?
There was a problem hiding this comment.
Yes, it's intentional. Where methods take a bool parameter and the calling of the method is a positive intent (i.e. enables something), a default parameter value of true should be passed. I thought we had a test for this, but the test is for bool?:
This should be updated to accept bool?. I think we also need a sweep before release to ensure we're adhering to this convention.
| @@ -238,7 +241,7 @@ public class HighlightFieldDescriptor<T> : DescriptorBase<HighlightFieldDescript | |||
| public HighlightFieldDescriptor<T> AllField() => this.Field("_all"); | |||
|
|
|||
| /// <inheritdoc/> | |||
| public HighlightFieldDescriptor<T> TagsSchema(string schema = "styled") => Assign(a => a.TagsSchema = schema); | |||
| public HighlightFieldDescriptor<T> TagsSchema(HighlighterTagsSchema? schema) => Assign(a => a.TagsSchema = schema); | |||
There was a problem hiding this comment.
I can make it the default, but the client generally doesn't have default values for nullable enums. I suspect this had a default value more as a hint for the accepted string value.
|
Changed the parameter to a |
Requires backporting to 5.x and 2.x in a non-binary breaking way.
Closes #2851