-
Notifications
You must be signed in to change notification settings - Fork 191
Conversation
@@ -26,7 +26,7 @@ private static IEnumerable<string> GetHeaderSplitImplementation(StringValues val | |||
{ | |||
foreach (var segment in new HeaderSegmentCollection(values)) | |||
{ | |||
if (segment.Data.HasValue) | |||
if (segment.Data.HasValue && !string.IsNullOrWhiteSpace(segment.Data.Value)) |
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.
This would be better implemented in StringSegment in M.E.Primitives.
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.
We have a separate bug to replace all IsNullOrWhiteSpace calls with IsNullOrEmpty. IsNullOrWhiteSpace checks for a bunch of unicode characters we don't want to check.
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.
And yes, I'm working on adding a bunch of methods to StringSegment: dotnet/extensions#213, you may want to hold off on this change.
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.
Commited.
|
||
Assert.Equal(segments, result); | ||
Assert.Equal(expectedResult, result); |
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.
Any reason we wanted to test keeping empty values before @Tratcher?
@@ -26,7 +26,7 @@ private static IEnumerable<string> GetHeaderSplitImplementation(StringValues val | |||
{ | |||
foreach (var segment in new HeaderSegmentCollection(values)) | |||
{ | |||
if (segment.Data.HasValue) | |||
if (segment.Data.HasValue && !string.IsNullOrWhiteSpace(segment.Data.Value)) |
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.
We have a separate bug to replace all IsNullOrWhiteSpace calls with IsNullOrEmpty. IsNullOrWhiteSpace checks for a bunch of unicode characters we don't want to check.
@@ -26,7 +26,7 @@ private static IEnumerable<string> GetHeaderSplitImplementation(StringValues val | |||
{ | |||
foreach (var segment in new HeaderSegmentCollection(values)) | |||
{ | |||
if (segment.Data.HasValue) | |||
if (segment.Data.HasValue && !string.IsNullOrWhiteSpace(segment.Data.Value)) |
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.
And yes, I'm working on adding a bunch of methods to StringSegment: dotnet/extensions#213, you may want to hold off on this change.
🆙📅 |
@@ -48,8 +51,9 @@ public void EmptyHeaderSegmentsAreParsable(IEnumerable<string> segments) | |||
}); | |||
|
|||
var result = headers.GetCommaSeparatedValues("Header1"); | |||
var expectedResult = segments.Where(s => !string.IsNullOrWhiteSpace(s)); |
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.
IsNullOrEmpty
@@ -26,7 +26,7 @@ private static IEnumerable<string> GetHeaderSplitImplementation(StringValues val | |||
{ | |||
foreach (var segment in new HeaderSegmentCollection(values)) | |||
{ | |||
if (segment.Data.HasValue) | |||
if (segment.Data.HasValue && !StringSegment.IsNullOrEmpty(segment.Data)) | |||
{ | |||
yield return DeQuote(segment.Data.Value); |
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.
What about empty quoted strings?
b4b5e6f
to
2a6d771
Compare
🆙📅 |
2a6d771
to
07470d4
Compare
Addresses #722. Only retain non-empty header values from GetCommaSeparatedValues.