add DisplayTable method #3098
add DisplayTable method #3098
Conversation
jonsequitur
force-pushed
the
DisplayTable
branch
from
86fb14d to
491d5a1
Compare
| @@ -226,22 +226,7 @@ void Initialize() | |||
| var formatter = GetDefaultFormatterForAnyObject(type); | |||
| return formatter.Format(value, context); | |||
| }), | |||
|
|
|||
| // Final last resort is to convert to plain text | |||
| new HtmlFormatter<object>((value, context) => | |||
There was a problem hiding this comment.
Was this removed because it was redundant?
There was a problem hiding this comment.
Yes. There were two different registrations for the same type. This one had no test coverage so it wasn't important.
| @@ -226,22 +226,7 @@ void Initialize() | |||
| var formatter = GetDefaultFormatterForAnyObject(type); | |||
There was a problem hiding this comment.
Looks like most lambdas in this collection don't guard against the value being null (although the removed last resort one below was checking for this). Is this intentional or does some other piece of code else already filter out nulls before these lambdas are invoked?
There was a problem hiding this comment.
Matching is done based on the runtime type and the formatter provides a universal early out for null.
| .Which | ||
| .Location | ||
| .SourceSpan | ||
| .LinePositionSpan.Start.Character |
There was a problem hiding this comment.
Considering creating a local for the LinePositionSpan so that the code to select the diagnostic need not be repeated across the 2 assertions.
| @@ -145,7 +146,69 @@ await kernel.FindKernelByName("csharp").As<CSharpKernel>() | |||
| [Theory] | |||
| [InlineData(Language.CSharp)] | |||
| [InlineData(Language.FSharp)] | |||
| public async Task Display_helper_can_be_called_without_specifying_class_name(Language language) | |||
| public async Task DisplayTable_produces_tabular_HTML_output_for_IEnumerable_T(Language language) | |||
There was a problem hiding this comment.
Should there be another test to validate what happens when the collection contains instances with heterogeneous types?
|
|
||
| _displayId = displayId; | ||
| _mimeTypes = new HashSet<string>(mimeTypes.Where(mimetype => !string.IsNullOrWhiteSpace(mimetype))); | ||
| FormattedValues = formattedValues ?? throw new ArgumentNullException(nameof(formattedValues)); |
There was a problem hiding this comment.
Should this also validate that there is at least one formatted value?
| } | ||
|
|
||
| public IReadOnlyCollection<string> MimeTypes => _mimeTypes; | ||
| public string DisplayId { get; } |
There was a problem hiding this comment.
Can this be private? Just checking since this seems like an implementation detail (for the DisplayedValue itself)
|
@jonsequitur I have gone ahead and merged this since none of my comments above seemed blocking (and can be addressed in a follow up PR) |
colombod
added
the
Area-Formatting
This addresses #3086.
It adds a new extension method,
IEnumerable<T>.DisplayTable, which can be used to display objects in a tabular format with the property names as headers. This implementation is unlike the original tabular format replaced in #2671 in the following ways:Trather than on the object instances, so instances of heterogeneous types in the sequence will not appear different from one another in the table.I've also made some changes to the
DisplayedValueclass, removing theMimeTypesproperty and instead storing the completeFormattedValueinstances, allowing inspection from the notebook, which was previously not possible:Finally, I've removed a duplicate
Diagnosticclass.