diff --git a/README.md b/README.md index 8ab3e65..f0b3188 100644 --- a/README.md +++ b/README.md @@ -51,9 +51,15 @@ Please see the [CONTRIBUTING.md](CONTRIBUTING.md) file for guidelines related to the Google's CLA, and code review requirements. As stated above the primary goal of this project is to develop readable, well -designed code, the functionality and efficiency comes second. With that please -expect rather more detailed code review. Please read the [design -guidelines](doc/design_guidelines.md) before contributing. +designed code, the functionality and efficiency come second. This is achieved +through detailed code reviews, design discussions and following of the [design +guidelines](doc/design_guidelines.md). Please familiarize yourself with these +before contributing. + +## Contributing widgets + +If you're developing a new widget, please see the [widget +development](doc/widget_development.md) section. # Project status diff --git a/canvas/testcanvas/testcanvas.go b/canvas/testcanvas/testcanvas.go index f437392..7bdb075 100644 --- a/canvas/testcanvas/testcanvas.go +++ b/canvas/testcanvas/testcanvas.go @@ -16,8 +16,8 @@ package testcanvas import ( + "fmt" "image" - "log" "github.com/mum4k/termdash/canvas" "github.com/mum4k/termdash/cell" @@ -28,7 +28,7 @@ import ( func MustNew(area image.Rectangle) *canvas.Canvas { cvs, err := canvas.New(area) if err != nil { - log.Fatalf("canvas.New => unexpected error: %v", err) + panic(fmt.Sprintf("canvas.New => unexpected error: %v", err)) } return cvs } @@ -36,13 +36,13 @@ func MustNew(area image.Rectangle) *canvas.Canvas { // MustApply applies the canvas on the terminal or panics. func MustApply(c *canvas.Canvas, t *faketerm.Terminal) { if err := c.Apply(t); err != nil { - log.Fatalf("canvas.Apply => unexpected error: %v", err) + panic(fmt.Sprintf("canvas.Apply => unexpected error: %v", err)) } } // MustSetCell sets the cell value or panics. func MustSetCell(c *canvas.Canvas, p image.Point, r rune, opts ...cell.Option) { if err := c.SetCell(p, r, opts...); err != nil { - log.Fatalf("canvas.SetCell => unexpected error: %v", err) + panic(fmt.Sprintf("canvas.SetCell => unexpected error: %v", err)) } } diff --git a/doc/design_guidelines.md b/doc/design_guidelines.md index acb7c5f..698c6eb 100644 --- a/doc/design_guidelines.md +++ b/doc/design_guidelines.md @@ -1,6 +1,21 @@ # Design guidelines -TODO(mum4k): Write the design guidelines, notes: -- low level draw functions in the draw package - each with unit tests and a - test utility. +## Don't clutter the widget code with drawing primitives +The widget implementations should contain high level code only. Low level +drawing primitives should be in separate packages. That way the widgets remain +easy to understand, enhance and test. + +E.g. the **gauge** widget contains code that calculates the size of the +rectangle that needs to be drawn. It doesn't contain code that draws the +rectangle itself as that belongs into the **draw** package. + +## Provide test helpers for all functions in the draw package + +To simplify unit tests of widgets, a test helper should be provided to all +functions in the **draw** package. + +E.g. a function called **Rectangle()** that draws a rectangle should come with +a helper caller **MustRectangle()**. Tests of a widget that uses +**Rectangle()** can just specify the expected rectangle by calling +**MustRectangle()** on the test canvas. diff --git a/draw/box.go b/draw/box.go index 7b705f4..b2843ab 100644 --- a/draw/box.go +++ b/draw/box.go @@ -44,7 +44,7 @@ func boxChar(p image.Point, box image.Rectangle, parts map[linePart]rune) rune { return -1 } -// Box draws a box on the canvas. +// Box draws a box (i.e. a border) on the canvas. func Box(c *canvas.Canvas, box image.Rectangle, ls LineStyle, opts ...cell.Option) error { if ar := c.Area(); !box.In(ar) { return fmt.Errorf("the requested box %+v falls outside of the provided canvas %+v", box, ar) diff --git a/draw/testdraw/testdraw.go b/draw/testdraw/testdraw.go index 59e62ef..f57188b 100644 --- a/draw/testdraw/testdraw.go +++ b/draw/testdraw/testdraw.go @@ -16,8 +16,8 @@ package testdraw import ( + "fmt" "image" - "log" "github.com/mum4k/termdash/canvas" "github.com/mum4k/termdash/cell" @@ -27,13 +27,13 @@ import ( // MustBox draws box on the canvas or panics. func MustBox(c *canvas.Canvas, box image.Rectangle, ls draw.LineStyle, opts ...cell.Option) { if err := draw.Box(c, box, ls, opts...); err != nil { - log.Fatalf("draw.Box => unexpected error: %v", err) + panic(fmt.Sprintf("draw.Box => unexpected error: %v", err)) } } // MustText draws the text on the canvas or panics. func MustText(c *canvas.Canvas, text string, tb draw.TextBounds, opts ...cell.Option) { if err := draw.Text(c, text, tb, opts...); err != nil { - log.Fatalf("draw.Text => unexpected error: %v", err) + panic(fmt.Sprintf("draw.Text => unexpected error: %v", err)) } } diff --git a/draw/text.go b/draw/text.go index 0560ca1..65d33d3 100644 --- a/draw/text.go +++ b/draw/text.go @@ -38,7 +38,9 @@ func (om OverrunMode) String() string { // overrunModeNames maps OverrunMode values to human readable names. var overrunModeNames = map[OverrunMode]string{ - OverrunModeStrict: "OverrunModeStrict", + OverrunModeStrict: "OverrunModeStrict", + OverrunModeTrim: "OverrunModeTrim", + OverrunModeThreeDot: "OverrunModeThreeDot", } const ( @@ -46,7 +48,12 @@ const ( // returns an error if it doesn't. OverrunModeStrict OverrunMode = iota - // TODO(mum4k): Support other overrun modes, like Trim, ThreeDot or LineWrap. + // OverrunModeTrim trims the part of the text that doesn't fit. + OverrunModeTrim + + // OverrunModeThreeDot trims the text and places the horizontal ellipsis + // '…' character at the end. + OverrunModeThreeDot ) // TextBounds specifies the limits (start and end cells) that the text must @@ -67,6 +74,27 @@ type TextBounds struct { Overrun OverrunMode } +// bounds enforces the text bounds based on the specified overrun mode. +// Returns test that can be safely drawn within the bounds. +func bounds(text string, maxRunes int, om OverrunMode) (string, error) { + runes := utf8.RuneCountInString(text) + if runes <= maxRunes { + return text, nil + } + + switch om { + case OverrunModeStrict: + return "", fmt.Errorf("the requested text %q takes %d runes to draw, space is available for only %d runes and overrun mode is %v", text, runes, maxRunes, om) + case OverrunModeTrim: + return text[:maxRunes], nil + + case OverrunModeThreeDot: + return fmt.Sprintf("%s…", text[:maxRunes-1]), nil + default: + return "", fmt.Errorf("unsupported overrun mode %v", om) + } +} + // Text prints the provided text on the canvas. func Text(c *canvas.Canvas, text string, tb TextBounds, opts ...cell.Option) error { ar := c.Area() @@ -84,13 +112,15 @@ func Text(c *canvas.Canvas, text string, tb TextBounds, opts ...cell.Option) err } else { wantMaxX = tb.MaxX } - runes := utf8.RuneCountInString(text) - if maxX := tb.Start.X + runes; maxX > wantMaxX && tb.Overrun == OverrunModeStrict { - return fmt.Errorf("the requested text %q would end at X coordinate %v which falls outside of the maximum %v", text, maxX, wantMaxX) + + maxRunes := wantMaxX - tb.Start.X + trimmed, err := bounds(text, maxRunes, tb.Overrun) + if err != nil { + return err } cur := tb.Start - for _, r := range text { + for _, r := range trimmed { if err := c.SetCell(cur, r, opts...); err != nil { return err } diff --git a/draw/text_test.go b/draw/text_test.go index 885421a..12ca945 100644 --- a/draw/text_test.go +++ b/draw/text_test.go @@ -45,6 +45,31 @@ func TestText(t *testing.T) { }, wantErr: true, }, + { + desc: "unsupported overrun mode specified", + canvas: image.Rect(0, 0, 1, 1), + text: "ab", + tb: TextBounds{ + Start: image.Point{0, 0}, + Overrun: OverrunMode(-1), + }, + want: func(size image.Point) *faketerm.Terminal { + return faketerm.MustNew(size) + }, + wantErr: true, + }, + { + desc: "zero text", + canvas: image.Rect(0, 0, 1, 1), + text: "", + tb: TextBounds{ + Start: image.Point{0, 0}, + Overrun: OverrunModeStrict, + }, + want: func(size image.Point) *faketerm.Terminal { + return faketerm.MustNew(size) + }, + }, { desc: "text falls outside of the canvas on OverrunModeStrict", canvas: image.Rect(0, 0, 1, 1), @@ -58,6 +83,76 @@ func TestText(t *testing.T) { }, wantErr: true, }, + { + desc: "text falls outside of the canvas on OverrunModeTrim", + canvas: image.Rect(0, 0, 1, 1), + text: "ab", + tb: TextBounds{ + Start: image.Point{0, 0}, + Overrun: OverrunModeTrim, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + c := testcanvas.MustNew(ft.Area()) + + testcanvas.MustSetCell(c, image.Point{0, 0}, 'a') + testcanvas.MustApply(c, ft) + return ft + }, + }, + { + desc: "OverrunModeTrim trims longer text", + canvas: image.Rect(0, 0, 2, 1), + text: "abcdef", + tb: TextBounds{ + Start: image.Point{0, 0}, + Overrun: OverrunModeTrim, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + c := testcanvas.MustNew(ft.Area()) + + testcanvas.MustSetCell(c, image.Point{0, 0}, 'a') + testcanvas.MustSetCell(c, image.Point{1, 0}, 'b') + testcanvas.MustApply(c, ft) + return ft + }, + }, + { + desc: "text falls outside of the canvas on OverrunModeThreeDot", + canvas: image.Rect(0, 0, 1, 1), + text: "ab", + tb: TextBounds{ + Start: image.Point{0, 0}, + Overrun: OverrunModeThreeDot, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + c := testcanvas.MustNew(ft.Area()) + + testcanvas.MustSetCell(c, image.Point{0, 0}, '…') + testcanvas.MustApply(c, ft) + return ft + }, + }, + { + desc: "OverrunModeThreeDot trims longer text", + canvas: image.Rect(0, 0, 2, 1), + text: "abcdef", + tb: TextBounds{ + Start: image.Point{0, 0}, + Overrun: OverrunModeThreeDot, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + c := testcanvas.MustNew(ft.Area()) + + testcanvas.MustSetCell(c, image.Point{0, 0}, 'a') + testcanvas.MustSetCell(c, image.Point{1, 0}, '…') + testcanvas.MustApply(c, ft) + return ft + }, + }, { desc: "requested MaxX is negative", canvas: image.Rect(0, 0, 1, 1), diff --git a/terminal/faketerm/faketerm.go b/terminal/faketerm/faketerm.go index 569a22b..1b0ee0f 100644 --- a/terminal/faketerm/faketerm.go +++ b/terminal/faketerm/faketerm.go @@ -85,7 +85,7 @@ func New(size image.Point, opts ...Option) (*Terminal, error) { func MustNew(size image.Point, opts ...Option) *Terminal { ft, err := New(size, opts...) if err != nil { - log.Fatalf("New => unexpected error: %v", err) + panic(fmt.Sprintf("New => unexpected error: %v", err)) } return ft } diff --git a/widgets/fakewidget/fakewidget.go b/widgets/fakewidget/fakewidget.go index abd6a12..fb07bb2 100644 --- a/widgets/fakewidget/fakewidget.go +++ b/widgets/fakewidget/fakewidget.go @@ -19,7 +19,6 @@ package fakewidget import ( "fmt" "image" - "log" "sync" "github.com/mum4k/termdash/area" @@ -183,6 +182,6 @@ func Draw(t terminalapi.Terminal, cvs *canvas.Canvas, opts widgetapi.Options, ev // MustDraw is like Draw, but panics on all errors. func MustDraw(t terminalapi.Terminal, cvs *canvas.Canvas, opts widgetapi.Options, events ...terminalapi.Event) { if err := Draw(t, cvs, opts, events...); err != nil { - log.Fatalf("Draw => %v", err) + panic(fmt.Sprintf("Draw => %v", err)) } }