Fixing racy behavior between Options and Draw.

This applies to widgets whose Options depend on user data.
Documenting this in the docs and on API and protecting against this
condition in the affected widgets.

container/draw.go

@@ -135,8 +135,7 @@ func drawResize(c *Container, area image.Rectangle) error {
 	if err != nil {
 		return err
 	}
-
-	if err := draw.Text(cvs, "⇄", image.Point{0, 0}); err != nil {
+	if err := draw.ResizeNeeded(cvs); err != nil {
 		return err
 	}
 	return cvs.Apply(c.term)

doc/widget_development.md

@@ -10,8 +10,8 @@ callers to set the displayed percentage.
 ## Thread safety
 
 All widget implementations must be thread safe, since the infrastructure calls
-the widget's **Draw()** method concurrently with the user of the widget setting
-the displayed values.
+the widget's **Options** and **Draw()** method concurrently with the user of
+the widget setting the displayed values.
 
 ## Drawing the widget's content
 
@@ -38,12 +38,18 @@ canvas in order to handle under sized or over sized terminals gracefully.
 If the current size of the terminal and the configured container splits result
 in a canvas smaller than the **MinimumSize**, the infrastructure won't call the
 widget's **Draw()** method. The widgets can use this to prevent impossible
-scenarios where an error would have to be returned.
+scenarios where an error would have to be returned. Note that if the values
+returned on a call to the **Options** method aren't static, but depend on the
+user data provided to the widget, the widget **must** protect against the
+scenario where the infrastructure provides a canvas that doesn't match the
+returned options. This is because the infrastructure cannot guarantee the user
+won't change the inputs between calls to **Options** and **Draw**.
 
 If the container configuration results in a canvas larger than **MaximumSize**
 the canvas will be limited to the specified size. Widgets can either specify a
 limit for both the maximum width and height or limit just one of them.
 
+
 ## Unit tests
 
 Unit tests utilize the **faketerm** package which is a fake implementation of a

draw/testdraw/testdraw.go

@@ -65,3 +65,10 @@ func MustBrailleCircle(bc *braille.Canvas, mid image.Point, radius int, opts ...
 		panic(fmt.Sprintf("draw.BrailleCircle => unexpected error: %v", err))
 	}
 }
+
+// MustResizeNeeded draws the character or panics.
+func MustResizeNeeded(cvs *canvas.Canvas) {
+	if err := draw.ResizeNeeded(cvs); err != nil {
+		panic(fmt.Sprintf("draw.ResizeNeeded => unexpected error: %v", err))
+	}
+}

draw/text.go

@@ -187,3 +187,9 @@ func Text(c *canvas.Canvas, text string, start image.Point, opts ...TextOption)
 	}
 	return nil
 }
+
+// ResizeNeeded draws an unicode character indicating that the canvas size is
+// too small to draw meaningful content.
+func ResizeNeeded(cvs *canvas.Canvas) error {
+	return Text(cvs, "⇄", image.Point{0, 0})
+}

draw/text_test.go

@@ -601,3 +601,56 @@ func TestText(t *testing.T) {
 		})
 	}
 }
+
+func TestResizeNeeded(t *testing.T) {
+	tests := []struct {
+		desc   string
+		canvas image.Rectangle
+		want   func(size image.Point) *faketerm.Terminal
+	}{
+		{
+			desc:   "draws the resize needed character",
+			canvas: image.Rect(0, 0, 1, 1),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				cvs := testcanvas.MustNew(ft.Area())
+				testcanvas.MustSetCell(cvs, image.Point{0, 0}, '⇄')
+				testcanvas.MustApply(cvs, ft)
+				return ft
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			cvs, err := canvas.New(tc.canvas)
+			if err != nil {
+				t.Fatalf("canvas.New => unexpected error: %v", err)
+			}
+
+			if err := ResizeNeeded(cvs); err != nil {
+				t.Fatalf("ResizeNeeded => unexpected error: %v", err)
+			}
+
+			got, err := faketerm.New(cvs.Size())
+			if err != nil {
+				t.Fatalf("faketerm.New => unexpected error: %v", err)
+			}
+			if err := cvs.Apply(got); err != nil {
+				t.Fatalf("Apply => unexpected error: %v", err)
+			}
+
+			want, err := faketerm.New(cvs.Size())
+			if err != nil {
+				t.Fatalf("faketerm.New => unexpected error: %v", err)
+			}
+			if tc.want != nil {
+				want = tc.want(cvs.Size())
+			}
+
+			if diff := faketerm.Diff(want, got); diff != "" {
+				t.Errorf("ResizeNeeded => %v", diff)
+			}
+		})
+	}
+}

widgetapi/widgetapi.go

@@ -83,5 +83,12 @@ type Widget interface {
 	// This is how the widget indicates to the infrastructure whether it is
 	// interested in keyboard or mouse shortcuts, what is its minimum canvas
 	// size, etc.
+	//
+	// Most widgets will return statically compiled options (minimum and
+	// maximum size, etc.). If the returned options depend on runtime state
+	// (e.g. the user data provided to the widget), the widget cannot depend on
+	// the infrastructure to no call the Draw method with a canvas that doesn't
+	// meet the requested options. This is because the data in the widget might
+	// change between calls to Options and Draw.
 	Options() Options
 }

widgets/barchart/barchart.go

@@ -23,6 +23,7 @@ import (
 	"sync"
 
 	"github.com/mum4k/termdash/align"
+	"github.com/mum4k/termdash/area"
 	"github.com/mum4k/termdash/canvas"
 	"github.com/mum4k/termdash/cell"
 	"github.com/mum4k/termdash/draw"
@@ -68,6 +69,14 @@ func (bc *BarChart) Draw(cvs *canvas.Canvas) error {
 	bc.mu.Lock()
 	defer bc.mu.Unlock()
 
+	needAr, err := area.FromSize(bc.minSize())
+	if err != nil {
+		return err
+	}
+	if !needAr.In(cvs.Area()) {
+		return draw.ResizeNeeded(cvs)
+	}
+
 	for i, v := range bc.values {
 		r, err := bc.barRect(cvs, i, v)
 		if err != nil {

widgets/barchart/barchart_test.go

@@ -107,6 +107,24 @@ func TestGauge(t *testing.T) {
 			},
 			wantUpdateErr: true,
 		},
+		{
+			desc: "draws resize needed character when canvas is smaller than requested",
+			bc: New(
+				Char('o'),
+			),
+			update: func(bc *BarChart) error {
+				return bc.Values([]int{0, 2, 5, 10}, 10)
+			},
+			canvas: image.Rect(0, 0, 1, 1),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				testdraw.MustResizeNeeded(c)
+				testcanvas.MustApply(c, ft)
+				return ft
+			},
+		},
 		{
 			desc: "displays bars",
 			bc: New(

widgets/gauge/gauge.go

@@ -249,6 +249,14 @@ func (g *Gauge) Draw(cvs *canvas.Canvas) error {
 	g.mu.Lock()
 	defer g.mu.Unlock()
 
+	needAr, err := area.FromSize(g.minSize())
+	if err != nil {
+		return err
+	}
+	if !needAr.In(cvs.Area()) {
+		return draw.ResizeNeeded(cvs)
+	}
+
 	if g.hasBorder() {
 		if err := draw.Border(cvs, cvs.Area(),
 			draw.BorderLineStyle(g.opts.border),

widgets/gauge/gauge_test.go

@@ -74,6 +74,23 @@ func TestGauge(t *testing.T) {
 				return ft
 			},
 		},
+		{
+			desc: "draws resize needed character when canvas is smaller than requested",
+			gauge: New(
+				Char('o'),
+				Border(draw.LineStyleLight),
+			),
+			percent: &percentCall{p: 35},
+			canvas:  image.Rect(0, 0, 1, 1),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				testdraw.MustResizeNeeded(c)
+				testcanvas.MustApply(c, ft)
+				return ft
+			},
+		},
 		{
 			desc: "aligns the progress text top and left",
 			gauge: New(

widgets/linechart/linechart.go

@@ -22,6 +22,7 @@ import (
 	"sort"
 	"sync"
 
+	"github.com/mum4k/termdash/area"
 	"github.com/mum4k/termdash/canvas"
 	"github.com/mum4k/termdash/canvas/braille"
 	"github.com/mum4k/termdash/cell"
@@ -173,6 +174,14 @@ func (lc *LineChart) Draw(cvs *canvas.Canvas) error {
 	lc.mu.Lock()
 	defer lc.mu.Unlock()
 
+	needAr, err := area.FromSize(lc.minSize())
+	if err != nil {
+		return err
+	}
+	if !needAr.In(cvs.Area()) {
+		return draw.ResizeNeeded(cvs)
+	}
+
 	yd, err := lc.yAxis.Details(cvs.Area(), lc.opts.yAxisMode)
 	if err != nil {
 		return fmt.Errorf("lc.yAxis.Details => %v", err)
@@ -285,19 +294,24 @@ func (lc *LineChart) Mouse(m *terminalapi.Mouse) error {
 	return errors.New("the LineChart widget doesn't support mouse events")
 }
 
-// Options implements widgetapi.Widget.Options.
-func (lc *LineChart) Options() widgetapi.Options {
-	lc.mu.Lock()
-	defer lc.mu.Unlock()
-
+// minSize determines the minimum required size to draw the line chart.
+func (lc *LineChart) minSize() image.Point {
 	// At the very least we need:
 	// - n cells width for the Y axis and its labels as reported by it.
 	// - at least 1 cell width for the graph.
 	reqWidth := lc.yAxis.RequiredWidth() + 1
 	// - 2 cells height the X axis and its values and 2 for min and max labels on Y.
 	const reqHeight = 4
+	return image.Point{reqWidth, reqHeight}
+}
+
+// Options implements widgetapi.Widget.Options.
+func (lc *LineChart) Options() widgetapi.Options {
+	lc.mu.Lock()
+	defer lc.mu.Unlock()
+
 	return widgetapi.Options{
-		MinimumSize: image.Point{reqWidth, reqHeight},
+		MinimumSize: lc.minSize(),
 	}
 }
 

widgets/linechart/linechart_test.go

@@ -64,14 +64,16 @@ func TestLineChartDraws(t *testing.T) {
 			wantWriteErr: true,
 		},
 		{
-			desc:    "draw fails when canvas not wide enough",
-			canvas:  image.Rect(0, 0, 2, 4),
-			wantErr: true,
+			desc:   "draws resize needed character when canvas is smaller than requested",
+			canvas: image.Rect(0, 0, 1, 1),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				testdraw.MustResizeNeeded(c)
+				testcanvas.MustApply(c, ft)
+				return ft
 			},
-		{
-			desc:    "draw fails when canvas not tall enough",
-			canvas:  image.Rect(0, 0, 3, 3),
-			wantErr: true,
 		},
 		{
 			desc:   "empty without series",

widgets/sparkline/sparkline.go

@@ -21,6 +21,7 @@ import (
 	"image"
 	"sync"
 
+	"github.com/mum4k/termdash/area"
 	"github.com/mum4k/termdash/canvas"
 	"github.com/mum4k/termdash/cell"
 	"github.com/mum4k/termdash/draw"
@@ -62,6 +63,14 @@ func (sl *SparkLine) Draw(cvs *canvas.Canvas) error {
 	sl.mu.Lock()
 	defer sl.mu.Unlock()
 
+	needAr, err := area.FromSize(sl.minSize())
+	if err != nil {
+		return err
+	}
+	if !needAr.In(cvs.Area()) {
+		return draw.ResizeNeeded(cvs)
+	}
+
 	ar := sl.area(cvs)
 	visible, max := visibleMax(sl.data, ar.Dx())
 	var curX int

widgets/sparkline/sparkline_test.go

@@ -291,6 +291,24 @@ func TestSparkLine(t *testing.T) {
 				return ft
 			},
 		},
+		{
+			desc: "draws resize needed character when canvas is smaller than requested",
+			sparkLine: New(
+				Height(2),
+			),
+			update: func(sl *SparkLine) error {
+				return sl.Add([]int{0, 100, 50, 85})
+			},
+			canvas: image.Rect(0, 0, 1, 1),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				testdraw.MustResizeNeeded(c)
+				testcanvas.MustApply(c, ft)
+				return ft
+			},
+		},
 		{
 			desc: "respects fixed height with label",
 			sparkLine: New(