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.
bevzzz · Jan 21, 2024 · e2ed872 · e2ed872
1 parent ee8a2df
commit e2ed872
Show file tree

Hide file tree

Showing 12 changed files with 505 additions and 293 deletions.
diff --git 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
@@ -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
@@ -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, "<pre>")
 	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, "<img src=\"data:")
-	io.WriteString(w, string(cell.Type()))
+	io.WriteString(w, string(cell.MimeType()))
 	io.WriteString(w, ";base64, ")
 	w.Write(cell.Text())
 	io.WriteString(w, "\" />\n")

diff --git 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", "<h1>Hi, mom!</h1>"),
+				cell: test.Raw("<h1>Hi, mom!</h1>", "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
@@ -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