Repository: exercism/cli
Pull request #473 (2017-08-21 20:31:24)

[nextercism] Refactor interactive stuff into its own package
It was really bugging me that the workspace thing had user-facing / interactive stuff. I've sequestered this into a new package, `comms`, which I'll use to build out other interactive stuff (such as #461 and #462).

I also added tests for `Pick()` and `ReadSelection()` (now `Selection{}.Read()`) which were missing.


Patch:
diff --git a/cmd/open.go b/cmd/open.go
index c1b0cff..f23a49d 100644
--- a/cmd/open.go
+++ b/cmd/open.go
@@ -1,9 +1,11 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/exercism/cli/browser"
+	"github.com/exercism/cli/comms"
 	"github.com/exercism/cli/config"
 	"github.com/exercism/cli/workspace"
 	"github.com/spf13/cobra"
@@ -49,7 +51,10 @@ the solution you want to see on the website.
 			solutions = mine
 		}
 
-		sx := workspace.Solutions(solutions)
+		selection := comms.NewSelection()
+		for _, solution := range solutions {
+			selection.Items = append(selection.Items, solution)
+		}
 		for {
 			prompt := `
 We found more than one. Which one did you mean?
@@ -57,13 +62,17 @@ Type the number of the one you want to select.
 
 %s
 > `
-			s, err := sx.Pick(prompt)
+			option, err := selection.Pick(prompt)
 			if err != nil {
 				fmt.Println(err)
 				continue
 			}
-			browser.Open(s.URL)
-			break
+			solution, ok := option.(*workspace.Solution)
+			if ok {
+				browser.Open(solution.URL)
+				break
+			}
+			BailOnError(errors.New("should never happen"))
 		}
 
 	},
diff --git a/comms/selection.go b/comms/selection.go
new file mode 100644
index 0000000..5d76086
--- /dev/null
+++ b/comms/selection.go
@@ -0,0 +1,78 @@
+package comms
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strconv"
+	"strings"
+)
+
+// Selection wraps a list of items.
+// It is used for interactive communication.
+type Selection struct {
+	Items  []fmt.Stringer
+	Reader io.Reader
+	Writer io.Writer
+}
+
+// NewSelection prepares an empty collection for interactive input.
+func NewSelection() Selection {
+	return Selection{
+		Reader: os.Stdin,
+		Writer: os.Stdout,
+	}
+}
+
+// Pick lets a user interactively select an option from a list.
+func (sel Selection) Pick(prompt string) (fmt.Stringer, error) {
+	// If there's just one, then we're done here.
+	if len(sel.Items) == 1 {
+		return sel.Items[0], nil
+	}
+
+	fmt.Fprintf(sel.Writer, prompt, sel.Display())
+
+	n, err := sel.Read(sel.Reader)
+	if err != nil {
+		return nil, err
+	}
+
+	o, err := sel.Get(n)
+	if err != nil {
+		return nil, err
+	}
+	return o, nil
+}
+
+// Display shows a numbered list of the solutions to choose from.
+// The list starts at 1, since that seems better in a user interface.
+func (sel Selection) Display() string {
+	str := ""
+	for i, item := range sel.Items {
+		str += fmt.Sprintf("  [%d] %s\n", i+1, item)
+	}
+	return str
+}
+
+// Read reads the user's selection and converts it to a number.
+func (sel Selection) Read(r io.Reader) (int, error) {
+	reader := bufio.NewReader(r)
+	text, _ := reader.ReadString('\n')
+	n, err := strconv.Atoi(strings.TrimSpace(text))
+	if err != nil {
+		return 0, err
+	}
+	return n, nil
+}
+
+// Get returns the solution corresponding to the number.
+// The list starts at 1, since that seems better in a user interface.
+func (sel Selection) Get(n int) (fmt.Stringer, error) {
+	if n <= 0 || n > len(sel.Items) {
+		return nil, errors.New("we don't have that one")
+	}
+	return sel.Items[n-1], nil
+}
diff --git a/workspace/solutions.go b/workspace/solutions.go
index b285d0f..20228ab 100644
--- a/workspace/solutions.go
+++ b/workspace/solutions.go
@@ -1,15 +1,5 @@
 package workspace
 
-import (
-	"bufio"
-	"errors"
-	"fmt"
-	"io"
-	"os"
-	"strconv"
-	"strings"
-)
-
 // Solutions is a collection of solutions to interactively choose from.
 type Solutions []*Solution
 
@@ -26,54 +16,3 @@ func NewSolutions(paths []string) (Solutions, error) {
 	}
 	return solutions, nil
 }
-
-// Pick lets a user interactively select a solution from a list.
-func (sx Solutions) Pick(prompt string) (*Solution, error) {
-	// If there's just one, then we're done here.
-	if len(sx) == 1 {
-		return sx[0], nil
-	}
-
-	fmt.Printf(prompt, sx.Display())
-
-	n, err := sx.ReadSelection(os.Stdin)
-	if err != nil {
-		return &Solution{}, err
-	}
-
-	s, err := sx.Get(n)
-	if err != nil {
-		return &Solution{}, err
-	}
-	return s, nil
-}
-
-// Display shows a numbered list of the solutions to choose from.
-// The list starts at 1, since that seems better in a user interface.
-func (sx Solutions) Display() string {
-	str := ""
-	for i, s := range sx {
-		str += fmt.Sprintf("  [%d] %s\n", i+1, s)
-	}
-	return str
-}
-
-// ReadSelection reads the user's selection and converts it to a number.
-func (sx Solutions) ReadSelection(r io.Reader) (int, error) {
-	reader := bufio.NewReader(r)
-	text, _ := reader.ReadString('\n')
-	n, err := strconv.Atoi(strings.TrimSpace(text))
-	if err != nil {
-		return 0, err
-	}
-	return n, nil
-}
-
-// Get returns the solution corresponding to the number.
-// The list starts at 1, since that seems better in a user interface.
-func (sx Solutions) Get(n int) (*Solution, error) {
-	if n <= 0 || n > len(sx) {
-		return &Solution{}, errors.New("can't do that")
-	}
-	return sx[n-1], nil
-}


Test patch:
diff --git a/comms/selection_test.go b/comms/selection_test.go
new file mode 100644
index 0000000..ad5bcd0
--- /dev/null
+++ b/comms/selection_test.go
@@ -0,0 +1,126 @@
+package comms
+
+import (
+	"fmt"
+	"io/ioutil"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type thing struct {
+	name   string
+	rating int
+}
+
+func (t thing) String() string {
+	return fmt.Sprintf("%s (+%d)", t.name, t.rating)
+}
+
+var (
+	things = []thing{
+		thing{name: "water", rating: 10},
+		thing{name: "food", rating: 3},
+		thing{name: "music", rating: 0},
+	}
+)
+
+func TestSelectionDisplay(t *testing.T) {
+	// We have to manually add each thing to the options collection.
+	var sel Selection
+	for _, thing := range things {
+		sel.Items = append(sel.Items, thing)
+	}
+
+	display := "  [1] water (+10)\n  [2] food (+3)\n  [3] music (+0)\n"
+	assert.Equal(t, display, sel.Display())
+}
+
+func TestSelectionGet(t *testing.T) {
+	var sel Selection
+	for _, thing := range things {
+		sel.Items = append(sel.Items, thing)
+	}
+
+	_, err := sel.Get(0)
+	assert.Error(t, err)
+
+	o, err := sel.Get(1)
+	assert.NoError(t, err)
+	// We need to do a type assertion to access
+	// any non-stringer stuff.
+	t1 := o.(thing)
+	assert.Equal(t, "water", t1.name)
+
+	o, err = sel.Get(2)
+	assert.NoError(t, err)
+	t2 := o.(thing)
+	assert.Equal(t, "food", t2.name)
+
+	o, err = sel.Get(3)
+	assert.NoError(t, err)
+	t3 := o.(thing)
+	assert.Equal(t, "music", t3.name)
+
+	_, err = sel.Get(4)
+	assert.Error(t, err)
+}
+
+func TestSelectionRead(t *testing.T) {
+	var sel Selection
+	n, err := sel.Read(strings.NewReader("5"))
+	assert.NoError(t, err)
+	assert.Equal(t, 5, n)
+
+	_, err = sel.Read(strings.NewReader("abc"))
+	assert.Error(t, err)
+}
+
+func TestSelectionPick(t *testing.T) {
+	tests := []struct {
+		desc      string
+		selection Selection
+		things    []thing
+		expected  string
+	}{
+		{
+			desc: "autoselect the only one",
+			selection: Selection{
+				// it never hits the error,
+				// because it doesn't actually do
+				// the prompt and read response.
+				Reader: strings.NewReader("BOOM!"),
+			},
+			things: []thing{
+				thing{"hugs", 100},
+			},
+			expected: "hugs",
+		},
+		{
+			desc: "it picks the one corresponding to the selection",
+			selection: Selection{
+				Reader: strings.NewReader("2"),
+			},
+			things: []thing{
+				thing{"food", 10},
+				thing{"water", 3},
+				thing{"music", 0},
+			},
+			expected: "water",
+		},
+	}
+
+	for _, test := range tests {
+		test.selection.Writer = ioutil.Discard
+		for _, th := range test.things {
+			test.selection.Items = append(test.selection.Items, th)
+		}
+
+		item, err := test.selection.Pick("which one? %s")
+		assert.NoError(t, err)
+		th, ok := item.(thing)
+		assert.True(t, ok)
+		assert.Equal(t, test.expected, th.name)
+	}
+}
diff --git a/workspace/solutions_test.go b/workspace/solutions_test.go
index 400d7ff..65fdf6a 100644
--- a/workspace/solutions_test.go
+++ b/workspace/solutions_test.go
@@ -34,32 +34,3 @@ func TestNewSolutions(t *testing.T) {
 	_, err = NewSolutions(paths)
 	assert.Error(t, err)
 }
-
-func TestSolutions(t *testing.T) {
-	solutions := []*Solution{
-		{Track: "a", Exercise: "foo"},
-		{Track: "b", Exercise: "foo"},
-		{Track: "c", Exercise: "foo"},
-	}
-	sx := Solutions(solutions)
-	display := "  [1] a/foo\n  [2] b/foo\n  [3] c/foo\n"
-	assert.Equal(t, display, sx.Display())
-
-	_, err := sx.Get(0)
-	assert.Error(t, err)
-
-	a, err := sx.Get(1)
-	assert.NoError(t, err)
-	assert.Equal(t, "a", a.Track)
-
-	b, err := sx.Get(2)
-	assert.NoError(t, err)
-	assert.Equal(t, "b", b.Track)
-
-	c, err := sx.Get(3)
-	assert.NoError(t, err)
-	assert.Equal(t, "c", c.Track)
-
-	_, err = sx.Get(4)
-	assert.Error(t, err)
-}