Skip to content
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

add DisplayTable method #3098

Merged
merged 1 commit into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ Microsoft.DotNet.Interactive
public System.String ToString()
public Diagnostic WithLinePositionSpan(LinePositionSpan linePositionSpan)
public class DisplayedValue
.ctor(System.String displayId, System.String mimeType, KernelInvocationContext context)
.ctor(System.String displayId, System.String[] mimeTypes, KernelInvocationContext context)
public System.Collections.Generic.IReadOnlyCollection<System.String> MimeTypes { get;}
.ctor(System.Collections.Generic.IReadOnlyList<FormattedValue> formattedValues, KernelInvocationContext context)
public System.String DisplayId { get;}
public System.Collections.Generic.IReadOnlyList<FormattedValue> FormattedValues { get;}
public System.Void Update(System.Object updatedValue)
public class EventRoutingSlip : RoutingSlip
.ctor()
Expand Down Expand Up @@ -672,10 +672,6 @@ Microsoft.DotNet.Interactive.Http
Microsoft.DotNet.Interactive.Parsing
public class ActionDirectiveNode : DirectiveNode
public System.String ParentKernelName { get;}
public class Diagnostic
public Location Location { get;}
public System.String Message { get;}
public DiagnosticSeverity Severity { get;}
public enum DiagnosticSeverity : System.Enum, System.IComparable, System.IConvertible, System.IFormattable
Hidden=0
Info=1
Expand All @@ -687,22 +683,19 @@ Microsoft.DotNet.Interactive.Parsing
public System.String GetHelpForSymbol(System.CommandLine.Symbol symbol)
public System.Void Write(System.CommandLine.Help.HelpContext context)
public abstract class DirectiveNode : LanguageNode
public System.Collections.Generic.IEnumerable<Diagnostic> GetDiagnostics()
public System.Collections.Generic.IEnumerable<Microsoft.DotNet.Interactive.Diagnostic> GetDiagnostics()
public System.CommandLine.Parsing.ParseResult GetDirectiveParseResult()
public class DirectiveToken : SyntaxToken
public System.String DirectiveName { get;}
public class KernelNameDirectiveNode : DirectiveNode
public class LanguageNode : SyntaxNode
public System.String Name { get;}
public System.Collections.Generic.IEnumerable<Diagnostic> GetDiagnostics()
public System.Collections.Generic.IEnumerable<Microsoft.DotNet.Interactive.Diagnostic> GetDiagnostics()
public class LanguageSpecificParseResult
public static LanguageSpecificParseResult None { get;}
.ctor()
public System.Collections.Generic.IEnumerable<Diagnostic> GetDiagnostics()
public System.Collections.Generic.IEnumerable<Microsoft.DotNet.Interactive.Diagnostic> GetDiagnostics()
public class LanguageToken : SyntaxToken
public class Location
public Microsoft.CodeAnalysis.Text.TextSpan SourceSpan { get;}
public PolyglotSyntaxTree SourceTree { get;}
public class PolyglotSubmissionNode : SyntaxNode
public System.String DefaultLanguage { get;}
public class PolyglotSyntaxTree
Expand Down Expand Up @@ -731,7 +724,7 @@ Microsoft.DotNet.Interactive.Parsing
public SyntaxNode FindNode(Microsoft.CodeAnalysis.Text.TextSpan span)
public SyntaxNode FindNode(System.Int32 position)
public SyntaxToken FindToken(System.Int32 position)
public System.Collections.Generic.IEnumerable<Diagnostic> GetDiagnostics()
public System.Collections.Generic.IEnumerable<Microsoft.DotNet.Interactive.Diagnostic> GetDiagnostics()
public abstract class SyntaxNodeOrToken
public SyntaxNode Parent { get;}
public Microsoft.CodeAnalysis.Text.TextSpan Span { get;}
Expand Down Expand Up @@ -944,3 +937,4 @@ System
public static class DisplayExtensions
public static Microsoft.DotNet.Interactive.DisplayedValue Display(String[] mimeTypes)
public static Microsoft.DotNet.Interactive.DisplayedValue DisplayAs(String mimeType)
public static Microsoft.DotNet.Interactive.DisplayedValue DisplayTable<T>(String[] mimeTypes)
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
using Assent;
using FluentAssertions;
using FluentAssertions.Primitives;
using Microsoft.DotNet.Interactive.Formatting.Tests.Utility;
using System.Linq;
using System;

namespace Microsoft.DotNet.Interactive.Formatting.Tests;
namespace Microsoft.DotNet.Interactive.Formatting.Tests.Utility;

public static class AssertionExtensions
{
Expand Down
29 changes: 7 additions & 22 deletions src/Microsoft.DotNet.Interactive.Formatting/HtmlFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal static PocketView TagWithPlainTextStyling(
internal static FormatterMapByType FormattersForAnyObject;

internal static FormatterMapByType FormattersForAnyEnumerable =
new(typeof(HtmlFormatter<>), nameof(HtmlFormatter<object>.CreateTableFormatterForAnyEnumerable));
new(typeof(HtmlFormatter<>), nameof(HtmlFormatter<object>.CreateTreeViewFormatterForAnyEnumerable));

internal static readonly ITypeFormatter[] DefaultFormatters =
{
Expand Down Expand Up @@ -212,11 +212,11 @@ type.FullName is not null &&
return true;
}),

new HtmlFormatter<decimal>((value, context) =>
{
FormatAndStyleAsPlainText(value, context);
return true;
}),
new HtmlFormatter<decimal>((value, context) =>
{
FormatAndStyleAsPlainText(value, context);
return true;
}),

new HtmlFormatter<object>((value, context) =>
{
Expand All @@ -226,22 +226,7 @@ type.FullName is not null &&
var formatter = GetDefaultFormatterForAnyObject(type);
Copy link
Contributor

@shyamnamboodiripad shyamnamboodiripad Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matching is done based on the runtime type and the formatter provides a universal early out for null.

return formatter.Format(value, context);
}),

// Final last resort is to convert to plain text
new HtmlFormatter<object>((value, context) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this removed because it was redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There were two different registrations for the same type. This one had no test coverage so it wasn't important.

{
if (value is null)
{
FormatAndStyleAsPlainText(Formatter.NullString, context);
}
else
{
FormatAndStyleAsPlainText(value, context);
}

return true;
}),


new HtmlFormatter<JsonDocument>((doc, context) =>
{
doc.RootElement.FormatTo(context, MimeType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.AspNetCore.Html;
using Microsoft.DotNet.Interactive.Utility;
using static Microsoft.DotNet.Interactive.Formatting.PocketViewTags;
Expand Down Expand Up @@ -55,7 +56,7 @@ public override bool Format(

public override string MimeType => HtmlFormatter.MimeType;

internal static HtmlFormatter<T> CreateTableFormatterForAnyEnumerable()
internal static HtmlFormatter<T> CreateTreeViewFormatterForAnyEnumerable()
{
Func<T, IEnumerable> getKeys = null;
Func<T, IEnumerable> getValues = instance => (IEnumerable)instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,7 @@ internal class TabularDataFormatterSource : ITypeFormatterSource
{
public IEnumerable<ITypeFormatter> CreateTypeFormatters()
{
yield return new HtmlFormatter<TabularDataResource>((value, context) =>
{
context.RequireDefaultStyles();

IReadOnlyList<IHtmlContent> headers =
value.Schema
.Fields
.Select(f => (IHtmlContent)td(span(f.Name)))
.ToArray();

var (rowData, remainingCount) = value.Data
.TakeAndCountRemaining(Formatter.ListExpansionLimit, true);

var rows =
rowData
.Select(d => (IHtmlContent)tr(d.Select(v => td(v.Value))))
.ToList();

if (remainingCount > 0)
{
rows.Add(tr(td[colspan: $"{headers.Count}"](i($"({remainingCount} more)"))));
}

Html.Table(headers, rows).WriteTo(context);
});
yield return new HtmlFormatter<TabularDataResource>((value, context) => FormatHtml(context, value));

yield return new JsonFormatter<TabularDataResource>((value, context) =>
{
Expand Down Expand Up @@ -66,4 +42,29 @@ public IEnumerable<ITypeFormatter> CreateTypeFormatters()
}, context);
});
}

internal static void FormatHtml(FormatContext context, TabularDataResource value)
{
context.RequireDefaultStyles();

IReadOnlyList<IHtmlContent> headers =
value.Schema
.Fields
.Select(f => (IHtmlContent)td(span(f.Name)))
.ToArray();

var (rowData, remainingCount) = value.Data.TakeAndCountRemaining(Formatter.ListExpansionLimit, true);

var rows =
rowData
.Select(d => (IHtmlContent)tr(d.Select(v => td(v.Value))))
.ToList();

if (remainingCount > 0)
{
rows.Add(tr(td[colspan: $"{headers.Count}"](i($"({remainingCount} more)"))));
}

Html.Table(headers, rows).WriteTo(context);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Formatting;
using Microsoft.DotNet.Interactive.Formatting.Tests;
using Microsoft.DotNet.Interactive.Formatting.Tests.Utility;
using Microsoft.DotNet.Interactive.Tests.Utility;
using Xunit;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.CSharp;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Formatting.Tests;
using Microsoft.DotNet.Interactive.Formatting.Tests.Utility;
using Microsoft.DotNet.Interactive.FSharp;
using Microsoft.DotNet.Interactive.Tests;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Formatting;
using Microsoft.DotNet.Interactive.Formatting.Tests;
using Microsoft.DotNet.Interactive.Formatting.Tests.Utility;
using Microsoft.DotNet.Interactive.Tests;
using Microsoft.DotNet.Interactive.Tests.Utility;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Linq;
using System.Reactive.Linq;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.CSharp;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Formatting;
using Microsoft.DotNet.Interactive.Tests.Utility;
using Microsoft.DotNet.Interactive.Formatting.Tests.Utility;
using Xunit;
using Xunit.Abstractions;
using static Microsoft.DotNet.Interactive.Formatting.Tests.Tags;
Expand Down Expand Up @@ -116,7 +117,7 @@ public async Task String_is_rendered_as_plain_text_via_implicit_return(
[Theory]
[InlineData(Language.CSharp, "{ \"hello\": 123 ", "application/json")]
[InlineData(Language.CSharp, "<span class=\"test\">hello!&nbsp;</span>", "text/html")]
public async Task String_is_rendered_as_specified_mime_type_DisplayAs(
public async Task DisplayAs_renders_string_as_specified_mime_type(
Language language,
string stringValue,
string mimeType)
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be another test to validate what happens when the collection contains instances with heterogeneous types?

{
var kernel = CreateKernel(language, openTestingNamespaces: true);

var code = language switch
{
Language.CSharp => """
new[] {
new { Title = "Troll 2", Stars = 0.25 },
new { Title = "The Room", Stars = 0.4 } }.DisplayTable();
""",
Language.FSharp => """
type MovieRating = { Title: string; Stars: float }
let ratings =
[ { Title = "Troll 2"; Stars = 0.25 };
{ Title = "The Room"; Stars = 0.4 } ]
ratings.DisplayTable()
"""
};

var result = await kernel.SendAsync(new SubmitCode(code));

result.Events.Should().NotContainErrors();

result.Events
.Should().ContainSingle<DisplayedValueProduced>()
.Which
.FormattedValues.Should().ContainSingle()
.Which
.Value.Should().ContainEquivalentHtmlFragments("""
<table>
<thead>
<tr>
<td><span>Title</span></td>
<td><span>Stars</span></td>
</tr>
</thead>
<tbody>
<tr>
<td>Troll 2</td>
<td>
<div class="dni-plaintext">
<pre>0.25</pre>
</div>
</td>
</tr>
<tr>
<td>The Room</td>
<td>
<div class="dni-plaintext">
<pre>0.4</pre>
</div>
</td>
</tr>
</tbody>
</table>
""");
}

[Theory]
[InlineData(Language.CSharp)]
[InlineData(Language.FSharp)]
public async Task display_can_be_called_without_specifying_class_name(Language language)
{
var kernel = CreateKernel(language, openTestingNamespaces: true);

Expand Down Expand Up @@ -189,7 +252,6 @@ public async Task Displayed_value_can_be_updated(Language language)
v.MimeType == "text/html" &&
v.Value.ToString().Contains("<b>hello</b>"));


KernelEvents
.OfType<DisplayedValueUpdated>()
.SelectMany(v => v.FormattedValues)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,24 @@ public void Directive_parsing_errors_are_available_as_diagnostics()
.Should()
.ContainSingle<DirectiveNode>()
.Which;
node
.GetDiagnostics()

var diagnostics = node.GetDiagnostics();

diagnostics
.Should()
.ContainSingle(d => d.Severity == CodeAnalysis.DiagnosticSeverity.Error)
.Which
.LinePositionSpan.End.Character
.Should()
.Be(node.Span.End);

diagnostics
.Should()
.ContainSingle(d => d.Severity == DiagnosticSeverity.Error)
.ContainSingle(d => d.Severity == CodeAnalysis.DiagnosticSeverity.Error)
.Which
.Location
.SourceSpan
.LinePositionSpan.Start.Character
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering creating a local for the LinePositionSpan so that the code to select the diagnostic need not be repeated across the 2 assertions.

.Should()
.BeEquivalentTo(node.Span);
.Be(node.Span.Start);
}

[Theory]
Expand Down
Loading