From e2ed872112053cc645ba9129da356746acab3c85 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Sat, 20 Jan 2024 19:28:05 +0100 Subject: [PATCH] feat: migrate to Preference API - Register CellRenderFuncs with render.Pref, which allows specifying both the cell type and the mime-type that the function should handle. - Replace schema.CellTypeMixed with distinct Type() and MimeType() fields. The legacy solution blended the two properties, which made the public API for registerring RenderCellFuncs rigid and confusing. - Move reusable test doubles to internal/test. After some more polishing I'm planning to expose them as a separate package to be used by extension packages in their tests. --- convert.go | 2 +- decode/decode_test.go | 2 +- render/html/html.go | 32 +++--- render/html/html_test.go | 152 ++++------------------------ render/html/wrapper.go | 8 +- render/html/wrapper_test.go | 55 ++++------- render/internal/test/cell.go | 97 ++++++++++++++++++ render/render.go | 186 ++++++++++++++++++++++++++++++----- render/render_test.go | 176 +++++++++++++++++++++++++++++++++ schema/notebook.go | 20 ---- schema/schema.go | 5 +- schema/v4/schema.go | 63 ++---------- 12 files changed, 505 insertions(+), 293 deletions(-) create mode 100644 render/internal/test/cell.go create mode 100644 render/render_test.go delete mode 100644 schema/notebook.go diff --git a/convert.go b/convert.go index 6ec265d..0b0b15e 100644 --- a/convert.go +++ b/convert.go @@ -57,7 +57,7 @@ func New(opts ...Option) *Notebook { // DefaultRenderer configures an HTML renderer. func DefaultRenderer() render.Renderer { - return render.New( + return render.NewRenderer( render.WithCellRenderers(html.NewRenderer()), ) } diff --git a/decode/decode_test.go b/decode/decode_test.go index fb6a90d..6d5280a 100644 --- a/decode/decode_test.go +++ b/decode/decode_test.go @@ -391,7 +391,7 @@ func TestDecodeBytes(t *testing.T) { // checkCell compares the cell's type and content to expected. func checkCell(tb testing.TB, got schema.Cell, want Cell) { tb.Helper() - require.Equalf(tb, want.Type, got.CellType(), "reported cell type: want %q, got %q", want.Type, got.CellType()) + require.Equalf(tb, want.Type, got.Type(), "reported cell type: want %q, got %q", want.Type, got.Type()) require.Equal(tb, want.MimeType, got.MimeType(), "reported mime type") if got, want := got.Text(), want.Text; !bytes.Equal(want, got) { tb.Errorf("text:\n(+want) %q\n(-got) %q", want, got) diff --git a/render/html/html.go b/render/html/html.go index 1a3daa4..9b83eb9 100644 --- a/render/html/html.go +++ b/render/html/html.go @@ -6,6 +6,7 @@ import ( "github.com/bevzzz/nb/render" "github.com/bevzzz/nb/schema" + "github.com/bevzzz/nb/schema/common" ) type Config struct { @@ -28,8 +29,7 @@ type Renderer struct { cfg Config } -// NewRenderer configures a new HTML renderer. -// By default, it embeds a *Wrapper and will panic if it is set to nil by one of the options. +// NewRenderer configures a new HTML renderer and embeds a *Wrapper to implement render.CellWrapper. func NewRenderer(opts ...Option) *Renderer { var cfg Config for _, opt := range opts { @@ -43,18 +43,23 @@ func NewRenderer(opts ...Option) *Renderer { } } -func (r *Renderer) RegisterFuncs(reg render.RenderCellFuncRegisterer) { - reg.Register(schema.MarkdownCellType, r.renderMarkdown) - reg.Register(schema.CodeCellType, r.renderCode) - reg.Register(schema.PNG, r.renderImage) - reg.Register(schema.JPEG, r.renderImage) - reg.Register(schema.HTML, r.renderRawHTML) - reg.Register(schema.JSON, r.renderRaw) - reg.Register(schema.StdoutCellType, r.renderRaw) - reg.Register(schema.StderrCellType, r.renderRaw) - reg.Register(schema.PlainTextCellType, r.renderRaw) +func (r *Renderer) RegisterFuncs(reg render.RenderCellFuncRegistry) { + // r.renderMarkdown should provide exact MimeType to override "text/*". + reg.Register(render.Pref{Type: schema.Markdown, MimeType: common.MarkdownText}, r.renderMarkdown) + reg.Register(render.Pref{Type: schema.Code}, r.renderCode) + + // Stream (stdout+stderr) and "error" outputs. + reg.Register(render.Pref{Type: schema.Stream}, r.renderRaw) + reg.Register(render.Pref{MimeType: common.Stderr}, r.renderRaw) // renders both "error" output and "stderr" stream + + // Various types of raw cell contents and display_data/execute_result outputs. + reg.Register(render.Pref{MimeType: "application/json"}, r.renderRaw) + reg.Register(render.Pref{MimeType: "text/*"}, r.renderRaw) + reg.Register(render.Pref{MimeType: "text/html"}, r.renderRawHTML) + reg.Register(render.Pref{MimeType: "image/*"}, r.renderImage) } +// renderMarkdown renders markdown cells as pre-formatted text. func (r *Renderer) renderMarkdown(w io.Writer, cell schema.Cell) error { io.WriteString(w, "
")
 	w.Write(cell.Text())
@@ -93,9 +98,10 @@ func (r *Renderer) renderRawHTML(w io.Writer, cell schema.Cell) error {
 	return nil
 }
 
+// renderImage writes base64-encoded image data.
 func (r *Renderer) renderImage(w io.Writer, cell schema.Cell) error {
 	io.WriteString(w, "\n")
diff --git a/render/html/html_test.go b/render/html/html_test.go
index 18b2525..6796ee8 100644
--- a/render/html/html_test.go
+++ b/render/html/html_test.go
@@ -11,35 +11,11 @@ import (
 
 	"github.com/bevzzz/nb/render"
 	"github.com/bevzzz/nb/render/html"
+	"github.com/bevzzz/nb/render/internal/test"
 	"github.com/bevzzz/nb/schema"
-	"github.com/bevzzz/nb/schema/common"
 )
 
 func TestRenderer(t *testing.T) {
-	t.Run("handles basic cell/mime types by default", func(t *testing.T) {
-		// Arrange
-		reg := make(funcRegistry)
-		r := html.NewRenderer()
-
-		// Act
-		r.RegisterFuncs(reg)
-
-		// Assert
-		for _, ct := range []schema.CellTypeMixed{
-			schema.CodeCellType,
-			schema.HTML,
-			schema.MarkdownCellType,
-			schema.JSON,
-			schema.PNG,
-			schema.JPEG,
-			schema.StdoutCellType,
-			schema.StderrCellType,
-			schema.PlainTextCellType,
-		} {
-			require.Contains(t, reg, ct, "expected a RenderCellFunc for cell type %q", ct)
-		}
-	})
-
 	t.Run("renders expected html", func(t *testing.T) {
 		for _, tt := range []struct {
 			name string
@@ -48,56 +24,56 @@ func TestRenderer(t *testing.T) {
 		}{
 			{
 				name: "markdown cell",
-				cell: markdown("# List:- One\n- Two\n -Three"),
+				cell: test.Markdown("# List:- One\n- Two\n -Three"),
 				want: &node{tag: "pre", content: "# List:- One\n- Two\n -Three"},
 			},
 			{
 				name: "raw text/html",
-				cell: raw("text/html", "

Hi, mom!

"), + cell: test.Raw("

Hi, mom!

", "text/html"), want: &node{tag: "h1", content: "Hi, mom!"}, }, { name: "raw text/plain", - cell: raw("text/html", "asdf"), + cell: test.Raw("asdf", "text/plain"), want: &node{tag: "pre", content: "asdf"}, }, { name: "application/json", - cell: displaydata("application/json", `{"one":1,"two":2}`), + cell: test.DisplayData(`{"one":1,"two":2}`, "application/json"), want: &node{tag: "pre", content: `{"one":1,"two":2}`}, }, { name: "stream to stdout", - cell: stdout("Two o'clock, and all's well!"), + cell: test.Stdout("Two o'clock, and all's well!"), want: &node{tag: "pre", content: "Two o'clock, and all's well!"}, }, { name: "stream to stderr", - cell: stderr("Mayday!Mayday!"), + cell: test.Stderr("Mayday!Mayday!"), want: &node{tag: "pre", content: "Mayday!Mayday!"}, }, { name: "image/png", - cell: displaydata("image/png", "base64-encoded-image"), + cell: test.DisplayData("base64-encoded-image", "image/png"), want: &node{tag: "img", attr: map[string][]string{ "src": {"data:image/png;base64, base64-encoded-image"}, }}, }, { name: "image/jpeg", - cell: displaydata("image/jpeg", "base64-encoded-image"), + cell: test.DisplayData("base64-encoded-image", "image/jpeg"), want: &node{tag: "img", attr: map[string][]string{ "src": {"data:image/jpeg;base64, base64-encoded-image"}, }}, }, { name: "code cell", - cell: &CodeCell{ - Cell: Cell{ - ct: schema.Code, - source: []byte("print('Hi, mom!')"), + cell: &test.CodeCell{ + Cell: test.Cell{ + CellType: schema.Code, + Source: []byte("print('Hi, mom!')"), }, - language: "python", + Lang: "python", }, want: &node{ tag: "div", @@ -131,17 +107,12 @@ func TestRenderer(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Arrange var buf bytes.Buffer - reg := make(funcRegistry) + r := render.NewRenderer() + reg := r.(render.RenderCellFuncRegistry) html.NewRenderer().RegisterFuncs(reg) - ct := tt.cell.Type() - rf, ok := reg[tt.cell.Type()] - if !ok { - t.Fatalf("no function registered for %q cell", ct) - } - // Act - err := rf(&buf, tt.cell) + err := r.Render(&buf, test.Notebook(tt.cell)) require.NoError(t, err) // Assert @@ -178,7 +149,7 @@ func TestRenderer_CSSWriter(t *testing.T) { } // Act - err = r.Wrap(io.Discard, markdown(""), noopRender) + err = r.Wrap(io.Discard, test.Markdown(""), noopRender) require.NoError(t, err) // Assert @@ -187,90 +158,3 @@ func TestRenderer_CSSWriter(t *testing.T) { } }) } - -// funcRegistry implements render.RenderCellFuncRegisterer for a plain map. -type funcRegistry map[schema.CellTypeMixed]render.RenderCellFunc - -var _ render.RenderCellFuncRegisterer = (*funcRegistry)(nil) - -func (r funcRegistry) Register(ct schema.CellTypeMixed, f render.RenderCellFunc) { - r[ct] = f -} - -func markdown(s string) schema.Cell { - return &Cell{ct: schema.Markdown, mimeType: common.MarkdownText, source: []byte(s)} -} - -func raw(mt string, s string) schema.Cell { - return &Cell{ct: schema.Raw, mimeType: mt, source: []byte(s)} -} - -func displaydata(mt string, s string) schema.Cell { - return &Cell{ct: schema.DisplayData, mimeType: mt, source: []byte(s)} -} - -func stdout(s string) schema.Cell { - return &Cell{ct: schema.Stream, mimeType: common.Stdout, source: []byte(s)} -} - -func stderr(s string) schema.Cell { - return &Cell{ct: schema.Stream, mimeType: common.Stderr, source: []byte(s)} -} - -// Cell is a test fixture to mock schema.Cell. -type Cell struct { - ct schema.CellType - mimeType string - source []byte -} - -var _ schema.Cell = (*Cell)(nil) - -func (c *Cell) CellType() schema.CellType { return c.ct } -func (c *Cell) MimeType() string { return c.mimeType } -func (c *Cell) Text() []byte { return c.source } - -// TODO: drop -func (c *Cell) Type() schema.CellTypeMixed { - switch c.ct { - case schema.Markdown: - return schema.MarkdownCellType - case schema.Code: - return schema.CodeCellType - case schema.Stream: - if c.mimeType == common.Stdout { - return schema.StdoutCellType - } - return schema.StderrCellType - } - return schema.CellTypeMixed(c.mimeType) -} - -// CodeCell is a test fixture to mock schema.CodeCell. -type CodeCell struct { - Cell - language string - executionCount int - outputs []schema.Cell -} - -var _ schema.CodeCell = (*CodeCell)(nil) - -func (code *CodeCell) Language() string { return code.language } -func (code *CodeCell) ExecutionCount() int { return code.executionCount } -func (code *CodeCell) Outputs() []schema.Cell { return code.outputs } - -// ExecuteResultOutput is a test fixture to mock cell outputs with ExecuteResult type. -type ExecuteResultOutput struct { - Cell - executionCount int -} - -var _ schema.Cell = (*ExecuteResultOutput)(nil) -var _ interface{ ExecutionCount() int } = (*ExecuteResultOutput)(nil) - -// TODO: drop -var _ interface{ TimesExecuted() int } = (*ExecuteResultOutput)(nil) - -func (ex *ExecuteResultOutput) ExecutionCount() int { return ex.executionCount } -func (ex *ExecuteResultOutput) TimesExecuted() int { return ex.executionCount } diff --git a/render/html/wrapper.go b/render/html/wrapper.go index e3b186e..667bfaa 100644 --- a/render/html/wrapper.go +++ b/render/html/wrapper.go @@ -31,7 +31,7 @@ func (wr *Wrapper) Wrap(w io.Writer, cell schema.Cell, render render.RenderCellF } var ct string - switch cell.CellType() { + switch cell.Type() { case schema.Markdown: ct = "jp-MarkdownCell" case schema.Code: @@ -68,8 +68,8 @@ func (wr *Wrapper) WrapInput(w io.Writer, cell schema.Cell, render render.Render } div.Close(w) - isCode := cell.CellType() == schema.Code - isMd := cell.CellType() == schema.Markdown + isCode := cell.Type() == schema.Code + isMd := cell.Type() == schema.Markdown if isCode { div.Open(w, attributes{ "class": { @@ -119,7 +119,7 @@ func (wr *Wrapper) WrapOutput(w io.Writer, cell schema.Outputter, render render. datamimetype = outs[0].MimeType() first := outs[0] - switch first.CellType() { + switch first.Type() { case schema.ExecuteResult: outputtypeclass = "jp-OutputArea-executeResult" child = true diff --git a/render/html/wrapper_test.go b/render/html/wrapper_test.go index 42f0a17..0c0cde5 100644 --- a/render/html/wrapper_test.go +++ b/render/html/wrapper_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/bevzzz/nb/render/html" + "github.com/bevzzz/nb/render/internal/test" "github.com/bevzzz/nb/schema" "github.com/bevzzz/nb/schema/common" ) @@ -26,7 +27,7 @@ func TestWrapper_Wrap(t *testing.T) { }{ { name: "markdown cell has jp-MarkdownCell class", - cell: markdown(""), + cell: test.Markdown(""), want: &node{ tag: "div", attr: map[string][]string{ @@ -40,7 +41,7 @@ func TestWrapper_Wrap(t *testing.T) { }, { name: "code cell has jp-CodeCell class", - cell: &Cell{ct: schema.Code}, + cell: &test.Cell{CellType: schema.Code}, want: &node{ tag: "div", attr: map[string][]string{ @@ -54,7 +55,7 @@ func TestWrapper_Wrap(t *testing.T) { }, { name: "raw cell has jp-RawCell class", - cell: &Cell{ct: schema.Raw}, + cell: test.Raw("", common.PlainText), want: &node{ tag: "div", attr: map[string][]string{ @@ -114,7 +115,7 @@ func TestWrapper_WrapInput(t *testing.T) { }{ { name: "markdown input", - cell: markdown(""), + cell: test.Markdown(""), want: &node{ tag: "div", attr: map[string][]string{ @@ -148,7 +149,7 @@ func TestWrapper_WrapInput(t *testing.T) { }, { name: "raw input", - cell: &Cell{ct: schema.Raw}, + cell: test.Raw("", common.PlainText), want: &node{ tag: "div", attr: map[string][]string{ @@ -171,9 +172,9 @@ func TestWrapper_WrapInput(t *testing.T) { }, { name: "code cell has a div additional classes and a non-empty prompt", - cell: &CodeCell{ - Cell: Cell{ct: schema.Code}, - executionCount: 10, + cell: &test.CodeCell{ + Cell: test.Cell{CellType: schema.Code}, + TimesExecuted: 10, }, want: &node{ tag: "div", @@ -275,7 +276,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "stream output to stdout", out: []schema.Cell{ - stdout(""), + test.Stdout(""), }, want: outputArea([]*node{ { @@ -300,7 +301,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "stream output to stderr", out: []schema.Cell{ - stderr(""), + test.Stderr(""), }, want: outputArea([]*node{ { @@ -325,7 +326,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "error output", out: []schema.Cell{ - &Cell{ct: schema.Error, mimeType: common.Stderr}, + test.ErrorOutput(""), }, want: outputArea([]*node{ { @@ -349,7 +350,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "display data image/png", out: []schema.Cell{ - displaydata("image/png", "base64-encoded-image"), + test.DisplayData("base64-encoded-image", "image/png"), }, want: outputArea([]*node{ { @@ -373,7 +374,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "display data image/jpeg", out: []schema.Cell{ - displaydata("image/jpeg", "base64-encoded-image"), + test.DisplayData("base64-encoded-image", "image/jpeg"), }, want: outputArea([]*node{ { @@ -397,14 +398,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "execute result text/html", out: []schema.Cell{ - &ExecuteResultOutput{ - Cell: Cell{ - ct: schema.ExecuteResult, - mimeType: "text/html", - source: []byte(``), - }, - executionCount: 10, - }, + test.ExecuteResult(``, "text/html", 10), }, want: outputArea([]*node{ { @@ -429,14 +423,7 @@ func TestWrapper_WrapOutput(t *testing.T) { { name: "execute result application/json", out: []schema.Cell{ - &ExecuteResultOutput{ - Cell: Cell{ - ct: schema.ExecuteResult, - mimeType: "application/json", - source: []byte(`{"one":1,"two":2}`), - }, - executionCount: 10, - }, + test.ExecuteResult(`{"one":1,"two":2}`, "application/json", 10), }, want: outputArea([]*node{ { @@ -724,13 +711,9 @@ func contains(s []string, v string) bool { return false } -// trimN trims 1 leading and 1 trailing character in cutset. +// trim1 trims 1 leading and 1 trailing character in cutset. func trim1(s string, cutset string) string { - if strings.HasPrefix(s, cutset) { - s = strings.TrimPrefix(s, cutset) - } - if strings.HasSuffix(s, cutset) { - s = strings.TrimSuffix(s, cutset) - } + s = strings.TrimPrefix(s, cutset) + s = strings.TrimSuffix(s, cutset) return s } diff --git a/render/internal/test/cell.go b/render/internal/test/cell.go new file mode 100644 index 0000000..250832f --- /dev/null +++ b/render/internal/test/cell.go @@ -0,0 +1,97 @@ +package test + +import ( + "github.com/bevzzz/nb/schema" + "github.com/bevzzz/nb/schema/common" +) + +// Markdown creates schema.Markdown cell with source s. +func Markdown(s string) schema.Cell { + return &Cell{CellType: schema.Markdown, Mime: common.MarkdownText, Source: []byte(s)} +} + +// Raw creates schema.Raw cell with source s and reported mime-type mt. +func Raw(s, mt string) schema.Cell { + return &Cell{CellType: schema.Raw, Mime: mt, Source: []byte(s)} +} + +// DisplayData creates schema.DisplayData cell with source s and reported mime-type mt. +func DisplayData(s, mt string) schema.Cell { + return &Cell{CellType: schema.DisplayData, Mime: mt, Source: []byte(s)} +} + +// ExecuteResult creates schema.ExecuteResult cell with source s, reported mime-type mt and execution count n. +func ExecuteResult(s, mt string, n int) schema.Cell { + return &ExecuteResultOutput{ + Cell: Cell{CellType: schema.ExecuteResult, Mime: mt, Source: []byte(s)}, + TimesExecuted: n, + } +} + +// ErrorOutput creates schema.Error cell with source s and mime-type common.Stderr. +func ErrorOutput(s string) schema.Cell { + return &Cell{CellType: schema.Error, Mime: common.Stderr, Source: []byte(s)} +} + +// Stdout creates schema.Stream cell with source s and mime-type common.Stdout. +func Stdout(s string) schema.Cell { + return &Cell{CellType: schema.Stream, Mime: common.Stdout, Source: []byte(s)} +} + +// Stderr creates schema.Stream cell with source s and mime-type common.Stderr. +func Stderr(s string) schema.Cell { + return &Cell{CellType: schema.Stream, Mime: common.Stderr, Source: []byte(s)} +} + +// Cell is a test fixture to mock schema.Cell. +type Cell struct { + CellType schema.CellType + Mime string // mime-type (avoid name-clash with the interface method) + Source []byte +} + +var _ schema.Cell = (*Cell)(nil) + +func (c *Cell) Type() schema.CellType { return c.CellType } +func (c *Cell) MimeType() string { return c.Mime } +func (c *Cell) Text() []byte { return c.Source } + +// CodeCell is a test fixture to mock schema.CodeCell. +// Use cases which only require schema.Cell, should create &test.Cell{CT: schema.Code} instead. +type CodeCell struct { + Cell + Lang string + TimesExecuted int + Out []schema.Cell +} + +var _ schema.CodeCell = (*CodeCell)(nil) + +func (code *CodeCell) Language() string { return code.Lang } +func (code *CodeCell) ExecutionCount() int { return code.TimesExecuted } +func (code *CodeCell) Outputs() []schema.Cell { return code.Out } + +// ExecuteResultOutput is a test fixture to mock cell outputs with ExecuteResult type. +type ExecuteResultOutput struct { + Cell + TimesExecuted int +} + +var _ schema.Cell = (*ExecuteResultOutput)(nil) +var _ interface{ ExecutionCount() int } = (*ExecuteResultOutput)(nil) + +func (ex *ExecuteResultOutput) ExecutionCount() int { return ex.TimesExecuted } + +// Notebook wraps a slice of cells into a simple schema.Notebook implementation. +func Notebook(cs ...schema.Cell) schema.Notebook { + return cells(cs) +} + +// cells implements schema.Notebook for a slice of cells. +type cells []schema.Cell + +var _ schema.Notebook = (*cells)(nil) + +func (n cells) Version() (v schema.Version) { return } + +func (n cells) Cells() []schema.Cell { return n } diff --git a/render/render.go b/render/render.go index 8be3db5..3582c58 100644 --- a/render/render.go +++ b/render/render.go @@ -3,8 +3,10 @@ package render import ( "fmt" "io" + "sort" "sync" + "github.com/bevzzz/nb/render/internal/wildcard" "github.com/bevzzz/nb/schema" ) @@ -32,13 +34,13 @@ type Renderer interface { // [Visitor]: https://refactoring.guru/design-patterns/visitor type CellRenderer interface { // RegisterFuncs registers one or more RenderCellFunc with the passed renderer. - RegisterFuncs(RenderCellFuncRegisterer) + RegisterFuncs(RenderCellFuncRegistry) } -// RenderCellFuncRegisterer is an interface that extendable Renderers should implement. -type RenderCellFuncRegisterer interface { - // Register adds a RenderCellFunc that will be called for cells of this type. - Register(schema.CellTypeMixed, RenderCellFunc) +// RenderCellFuncRegistry is an interface that extendable Renderers should implement. +type RenderCellFuncRegistry interface { + // Register adds a RenderCellFunc and a Pref selector for it. + Register(Pref, RenderCellFunc) } // RenderCellFunc writes contents of a specific cell type. @@ -76,52 +78,73 @@ type CellWrapper interface { // renderer is a base Renderer implementation. // It does not support any cell types out of the box and should be extended by the client using the available Options. type renderer struct { - once sync.Once - cellWrapper CellWrapper - renderCellFuncsTmp map[schema.CellTypeMixed]RenderCellFunc - renderCellFuncs map[schema.CellTypeMixed]RenderCellFunc + once sync.Once + cellWrapper CellWrapper + + renderCellFuncsTmp map[Pref]RenderCellFunc // funcPrefsTmp holds intermediary preference entries. + renderCellFuncs prefs // renderCellFuncs is sorted and will only be modified once. } -// New extends the base renderer with the passed options. -func New(opts ...Option) Renderer { +var _ RenderCellFuncRegistry = (*renderer)(nil) + +// NewRenderer extends the base renderer with the passed options. +func NewRenderer(opts ...Option) Renderer { r := renderer{ - renderCellFuncsTmp: make(map[schema.CellTypeMixed]RenderCellFunc), - renderCellFuncs: make(map[schema.CellTypeMixed]RenderCellFunc), cellWrapper: nil, + renderCellFuncsTmp: make(map[Pref]RenderCellFunc), } r.AddOptions(opts...) return &r } +var _ Renderer = (*renderer)(nil) +var _ RenderCellFuncRegistry = (*renderer)(nil) + func (r *renderer) AddOptions(opts ...Option) { for _, opt := range opts { opt(r) } } -// Register registers a new RenderCellFunc for the cell type. +// Register registers a new RenderCellFunc with a preference selector. // -// Any previously registered functions will be overridden. All configurations +// Any function registered with the same Pref will be overridden. All configurations // should be done the first call to Render(), as later changes will have no effect. -func (r *renderer) Register(t schema.CellTypeMixed, f RenderCellFunc) { - r.renderCellFuncsTmp[t] = f +func (r *renderer) Register(pref Pref, f RenderCellFunc) { + r.renderCellFuncsTmp[pref] = f } func (r *renderer) init() { r.once.Do(func() { - for t, f := range r.renderCellFuncsTmp { - r.renderCellFuncs[t] = f + for p, rf := range r.renderCellFuncsTmp { + r.renderCellFuncs = append(r.renderCellFuncs, pref{ + Pref: p, + Render: rf, + }) } + sort.Sort(r.renderCellFuncs) }) } -// render the contents of a cell if a RenderCellFunc is registered for its type. +// render renders the cell with the most-preferred RenderCellFunc. +// +// TODO: use sort.Find? need to try it out, like, because we have a mixed slice, where s[i] > s[i-1] might be true, but then s[i] and s[i-2] are semantically unrelated. +// Definitely not sort.Search, because sort.Search assumes that all elements >=i satisfy the condition, which is not the case. func (r *renderer) render(w io.Writer, cell schema.Cell) error { - render, ok := r.renderCellFuncs[cell.Type()] - if ok { - if err := render(w, cell); err != nil { - return fmt.Errorf("ipynb: render: %w", err) + for _, pref := range r.renderCellFuncs { + if !pref.Match(cell) { + continue + } + if err := pref.Render(w, cell); err != nil { + // We could implement a failover mechanism, where, if the first-preference render fails, + // we move on to the next matching option. The trouble here is that the first renderer + // couldn've already written to io.Writer and we might end up with a corrupted document. + // + // Using an intermediate buffer buf and copying from it to w on successful render is an option, + // but it adds some overhead and I wouldn't take it without a compelling case for this feature. + return fmt.Errorf("nb: render: %w", err) } + return nil } // TODO: currently we silently drop cells for which no render func is registered. Should we error? return nil @@ -133,6 +156,8 @@ func (r *renderer) Render(w io.Writer, nb schema.Notebook) error { for _, cell := range nb.Cells() { var err error + // TODO: lookup RenderCellFunc before opening the wrapper? + if r.cellWrapper != nil { err = r.cellWrapper.Wrap(w, cell, func(w io.Writer, c schema.Cell) error { if err := r.cellWrapper.WrapInput(w, cell, r.render); err != nil { @@ -156,3 +181,116 @@ func (r *renderer) Render(w io.Writer, nb schema.Notebook) error { } return nil } + +// Pref describes target cell and mime- type. +// +// Preference API is a flexible model which allows multiple CellRenderers +// to assume responsibility for specific cells. For example: +// +// // Default renderer handles all "display_data" outputs: media, JSON, raw HTML, etc. +// reg.Register(render.Pref{Type: schema.DisplayData}, r.renderDisplayData) +// +// // This custom renderer only renders GIFs (regardless of the cell type). +// reg.Register(render.Pref{MimeType: "image/gif"}, r.renderGIF) +// +// // Finally, this renderer renders any other image media, but only from "display_data" outputs. +// reg.Register(render.Pref{Type: schema.DisplayData, MimeType: "image/*"}, r.renderSQL) +// +// To provide this granularity, registered Prefs are sorted according to their: +// 1. Specificity: a measure for how precise the selection of target cells is. +// Simply put, Type < MimeType < (Type+MimeType). +// 2. Wildcard count: Prefs with less "*" in their MimeType will be prioritized. +type Pref struct { + // Type matches cells with the same Type(). + Type schema.CellType + + // MimeType matches cells based on their reported MimeType(). + // Use wildcard syntax (e.g. "image/*" or "*/*") to target + // wider ranges of cell mime-types. + MimeType string +} + +// Match checks if the cell matches Pref's criteria. +func (p Pref) Match(cell schema.Cell) bool { + if p.Type > schema.Unrecognized && p.Type != cell.Type() { + return false + } + if p.MimeType != "" && !wildcard.Match(p.MimeType, cell.MimeType()) { + return false + } + return true +} + +// specificity calculates a score for how precise the selection of target cells is. +// Generally, Prefs that define more fields achieve greater specificity. +// Below are some examples: +// - Type - (1) +// - MimeType - (2) +// - MimeType + Type - (3) +// +// A larger increment is used to make sure MimeType yields greater value than Type. +// The exact values should not be relied upon, as they may change in the future. +func (p Pref) specificity() (s int) { + if p.Type > schema.Unrecognized { + s++ + } + if p.MimeType != "" { + s += 2 + } + return +} + +// pref adds RenderCellFunc to Pref to keep Pref hashable. +type pref struct { + Pref + Render RenderCellFunc +} + +// prefs is a RenderCellFunc collection that sorts in the order of descending Pref specificity. +type prefs []pref + +var _ sort.Interface = (*prefs)(nil) + +// Len is the number of pref elements. +func (s prefs) Len() int { + return len(s) +} + +// Swap swaps 2 pref elements. +func (s prefs) Swap(i, j int) { + tmp := s[i] + s[i] = s[j] + s[j] = tmp +} + +// Less returns true if s[i] is more specific than s[j]. +func (s prefs) Less(i, j int) bool { + return less(s[i].Pref, s[j].Pref) +} + +// less returns true if p is more specific than other. It can be used to sort +// a slice of Prefs in the order of descending specificity: +// +// sort.Slice(len(prefs), func(i, j int) bool { +// return less(prefs[i], prefs[j]) +// }) +// +// In addition to specificity, less considers mime-type semantics. That is, if both Prefs +// have non-zero MimeType and target the same Type (regardless which), less returns true +// if the other Pref uses more wildcards in its mime-type (which makes it less specific). +// For example, "text/*" is less specific than "text/plain", but more specific than "*/*". +func less(p, other Pref) bool { + if s, sOther := p.specificity(), other.specificity(); s != sOther { + return s > sOther + } + + // Prefs that target different cell types are unrelated and can be sorted in any order. + if p.Type != other.Type { + return false + } + + // At this point we know both Prefs have a non-zero MimeType, + // otherwise their specificities would not be the same. Given that, + // p must sort before other iff its MimeType is more exact (uses less wildcards). + return wildcard.Count(p.MimeType) < wildcard.Count(other.MimeType) +} diff --git a/render/render_test.go b/render/render_test.go new file mode 100644 index 0000000..8ed9c42 --- /dev/null +++ b/render/render_test.go @@ -0,0 +1,176 @@ +package render_test + +import ( + "io" + "strings" + "testing" + + "github.com/bevzzz/nb/render" + "github.com/bevzzz/nb/render/internal/test" + "github.com/bevzzz/nb/schema" + "github.com/bevzzz/nb/schema/common" + "github.com/stretchr/testify/require" +) + +func TestRenderer_Render(t *testing.T) { + // More than anything, this test ensures that renderer + // will correctly deduplicate and prioritize RenderCellFuncs + // registered by extensions. That way, concrete CellRenderer implementations + // will only need to test that their Prefs capture all their target cells. + + r, ok := render.NewRenderer().(render.RenderCellFuncRegistry) + if !ok { + t.Errorf("%T does not implement render.RenderCellFuncRegisterer", r) + } + + // writeString returns a render.RenderCellFunc that writes s to w. + writeString := func(s string) render.RenderCellFunc { + return func(w io.Writer, c schema.Cell) error { + io.WriteString(w, s) + return nil + } + } + + for _, tt := range []struct { + name string + standard renderCellFuncs // standard functions immitate existing (default) render cell funcs + prefs renderCellFuncs // functions expected to be used in the Act step + cell schema.Cell + want string + }{ + { + name: "any markdown cell", + standard: renderCellFuncs{ + render.Pref{Type: schema.Markdown}: writeString("default markdown"), + }, + prefs: renderCellFuncs{ + render.Pref{Type: schema.Markdown}: writeString("custom markdown"), + }, + cell: test.Markdown(""), + want: "custom markdown", + }, + { + name: "exact mime-type overrides wildcard", + standard: renderCellFuncs{ + render.Pref{MimeType: "text/*"}: writeString("any text"), + }, + prefs: renderCellFuncs{ + render.Pref{MimeType: common.MarkdownText}: writeString("custom markdown"), + }, + cell: test.Markdown(""), + want: "custom markdown", + }, + { + name: "cell type + mime-type overrides exact mime-type", + standard: renderCellFuncs{ + render.Pref{MimeType: "image/png"}: writeString("any PNG image"), + }, + prefs: renderCellFuncs{ + render.Pref{Type: schema.DisplayData, MimeType: "image/png"}: writeString("display data PNG"), + }, + cell: test.DisplayData("", "image/png"), + want: "display data PNG", + }, + { + name: "mime-type will less wildcards is prioritized", + standard: renderCellFuncs{ + render.Pref{MimeType: "*/*"}: writeString("any mime-type"), + }, + prefs: renderCellFuncs{ + render.Pref{MimeType: "text/*"}: writeString("any text"), + }, + cell: test.Raw("", "text/html"), + want: "any text", + }, + } { + t.Run(tt.name, func(t *testing.T) { + // Arrange + r := render.NewRenderer() + reg := r.(render.RenderCellFuncRegistry) + tt.standard.RegisterFuncs(reg) + + tt.prefs.RegisterFuncs(reg) + var sb strings.Builder + + // Act + err := r.Render(&sb, test.Notebook(tt.cell)) + require.NoError(t, err) + + // Assert + if got := sb.String(); got != tt.want { + t.Errorf("wrong content: want %q, got %q", tt.want, got) + } + }) + } + +} + +// renderCellFuncs implements render.CellRenderer for a map[render.Pref]render.RenderCellFunc. +type renderCellFuncs map[render.Pref]render.RenderCellFunc + +var _ render.CellRenderer = new(renderCellFuncs) + +func (sr renderCellFuncs) RegisterFuncs(reg render.RenderCellFuncRegistry) { + for pref := range sr { + reg.Register(pref, sr[pref]) + } +} + +func TestPref_Match(t *testing.T) { + for _, tt := range []struct { + name string + pref render.Pref + wantMatch []schema.Cell + noMatch []schema.Cell + }{ + { + name: "only cell type", + pref: render.Pref{Type: schema.Markdown}, + wantMatch: []schema.Cell{test.Markdown("")}, + noMatch: []schema.Cell{ + test.Raw("", "text/markdown"), + &test.Cell{CellType: schema.Code}, + }, + }, + { + name: "only mime-type", + pref: render.Pref{MimeType: "image/*"}, + wantMatch: []schema.Cell{ + test.Raw("", "image/jpeg"), + test.DisplayData("", "image/png"), + test.ExecuteResult("", "image/svg+xml", 0), + }, + noMatch: []schema.Cell{ + test.Raw("", "text/html"), + test.Markdown(""), + test.Stdout(""), + }, + }, + { + name: "cell type and mime-type", + pref: render.Pref{Type: schema.Code, MimeType: "*/javascript"}, + wantMatch: []schema.Cell{ + &test.Cell{CellType: schema.Code, Mime: "text/javascript"}, + &test.Cell{CellType: schema.Code, Mime: "application/javascript"}, + }, + noMatch: []schema.Cell{ + &test.Cell{CellType: schema.Code, Mime: "text/js"}, + &test.Cell{CellType: schema.Code, Mime: "application/x+javascript"}, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + for _, cell := range tt.wantMatch { + if got := tt.pref.Match(cell); !got { + t.Errorf("%+v should match cell %+v", tt.pref, cell) + } + } + + for _, cell := range tt.noMatch { + if got := tt.pref.Match(cell); got { + t.Errorf("%+v should not match cell %+v", tt.pref, cell) + } + } + }) + } +} diff --git a/schema/notebook.go b/schema/notebook.go deleted file mode 100644 index b1340ac..0000000 --- a/schema/notebook.go +++ /dev/null @@ -1,20 +0,0 @@ -package schema - -type CellTypeMixed string - -const ( - // TODO: drop these - - PlainTextCellType CellTypeMixed = "text/plain" - MarkdownCellType CellTypeMixed = "text/markdown" - HTML CellTypeMixed = "text/html" - PNG CellTypeMixed = "image/png" - JPEG CellTypeMixed = "image/jpeg" - JSON CellTypeMixed = "application/json" - - // Internal cell types - - CodeCellType CellTypeMixed = "code" - StdoutCellType CellTypeMixed = "stdout" - StderrCellType CellTypeMixed = "stderr" -) diff --git a/schema/schema.go b/schema/schema.go index a1aaffb..15a38e9 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -29,12 +29,9 @@ type NotebookMetadata interface { // Cell encapsulates the raw content of each notebook cell and its designated mime-type. type Cell interface { - CellType() CellType + Type() CellType MimeType() string Text() []byte - - // Type will be superseded with CellType in the following commits. - Type() CellTypeMixed } // CellType reports the intended cell type to the components that work diff --git a/schema/v4/schema.go b/schema/v4/schema.go index 793c6ec..de1885b 100644 --- a/schema/v4/schema.go +++ b/schema/v4/schema.go @@ -65,7 +65,7 @@ type Markdown struct { var _ schema.Cell = (*Markdown)(nil) -func (md *Markdown) CellType() schema.CellType { +func (md *Markdown) Type() schema.CellType { return schema.Markdown } @@ -73,10 +73,6 @@ func (md *Markdown) MimeType() string { return common.MarkdownText } -func (md *Markdown) Type() schema.CellTypeMixed { - return schema.MarkdownCellType -} - func (md *Markdown) Text() []byte { return md.Source.Text() } @@ -89,7 +85,7 @@ type Raw struct { var _ schema.Cell = (*Raw)(nil) -func (raw *Raw) CellType() schema.CellType { +func (raw *Raw) Type() schema.CellType { return schema.Raw } @@ -97,10 +93,6 @@ func (raw *Raw) MimeType() string { return raw.Metadata.MimeType() } -func (raw *Raw) Type() schema.CellTypeMixed { - return raw.Metadata.Type() -} - func (raw *Raw) Text() []byte { return raw.Source.Text() } @@ -123,18 +115,6 @@ func (raw *RawCellMetadata) MimeType() string { } } -// Type returns a more specific mime-type if one is provided and "text/plain" otherwise. -func (raw *RawCellMetadata) Type() schema.CellTypeMixed { - switch { - case raw.Format != nil: - return schema.CellTypeMixed(*raw.Format) - case raw.RawMimeType != nil: - return schema.CellTypeMixed(*raw.RawMimeType) - default: - return schema.PlainTextCellType - } -} - // Code defines the schema for a "code" cell. type Code struct { Source common.MultilineString `json:"source"` @@ -146,7 +126,7 @@ type Code struct { var _ schema.CodeCell = (*Code)(nil) var _ schema.Outputter = (*Code)(nil) -func (code *Code) CellType() schema.CellType { +func (code *Code) Type() schema.CellType { return schema.Code } @@ -155,10 +135,6 @@ func (code *Code) MimeType() string { return "application/x-python" } -func (code *Code) Type() schema.CellTypeMixed { - return schema.CodeCellType -} - func (code *Code) Text() []byte { return code.Source.Text() } @@ -222,7 +198,7 @@ type StreamOutput struct { var _ schema.Cell = (*StreamOutput)(nil) -func (stream *StreamOutput) CellType() schema.CellType { +func (stream *StreamOutput) Type() schema.CellType { return schema.Stream } @@ -236,16 +212,6 @@ func (stream *StreamOutput) MimeType() string { return common.PlainText } -func (stream *StreamOutput) Type() schema.CellTypeMixed { - switch stream.Target { - case "stdout": - return schema.StdoutCellType - case "stderr": - return schema.StderrCellType - } - return "text/plain" -} - func (stream *StreamOutput) Text() []byte { return stream.Source.Text() } @@ -258,7 +224,7 @@ type DisplayDataOutput struct { var _ schema.Cell = (*DisplayDataOutput)(nil) -func (dd *DisplayDataOutput) CellType() schema.CellType { +func (dd *DisplayDataOutput) Type() schema.CellType { return schema.DisplayData } @@ -278,17 +244,6 @@ func (mb MimeBundle) MimeType() string { return common.PlainText } -// Type returns the richer of the mime-types present in the bundle, -// and falls back to "text/plain" otherwise. -func (mb MimeBundle) Type() schema.CellTypeMixed { - for t := range mb { - if schema.CellTypeMixed(t) != schema.PlainTextCellType { - return schema.CellTypeMixed(t) - } - } - return schema.PlainTextCellType -} - // Text returns data with the richer mime-type. func (mb MimeBundle) Text() []byte { return mb.Data(mb.MimeType()) @@ -330,7 +285,7 @@ type ExecuteResultOutput struct { var _ schema.Cell = (*ExecuteResultOutput)(nil) var _ schema.ExecutionCounter = (*ExecuteResultOutput)(nil) -func (ex *ExecuteResultOutput) CellType() schema.CellType { +func (ex *ExecuteResultOutput) Type() schema.CellType { return schema.ExecuteResult } @@ -347,7 +302,7 @@ type ErrorOutput struct { var _ schema.Cell = (*ErrorOutput)(nil) -func (err *ErrorOutput) CellType() schema.CellType { +func (err *ErrorOutput) Type() schema.CellType { return schema.Error } @@ -355,10 +310,6 @@ func (err *ErrorOutput) MimeType() string { return common.Stderr } -func (err *ErrorOutput) Type() schema.CellTypeMixed { - return schema.StderrCellType -} - func (err *ErrorOutput) Text() (txt []byte) { s := strings.Join(err.Traceback, "\n") return []byte(s)