Repository: exercism/cli
Pull request #474 (2017-08-22 20:58:05)

Create small abstraction for interactive -uh- interactions
If someone tries downloading or submitting something before
they've configured their token or workspace, we're going to want
to ask them to paste it to us.

For the workspace we want to be able to provide a default suggestion.

This Question abstraction provides a very small wrapper around such
an interaction, and makes it testable by providing the Reader/Writer
which can be overwritten with pre-populated readers, or a writer that
discards everything.

I wonder if something like this would make the basis for a good Exercism
exercise. It took me a while to figure out how to make this both clean
and testable. E.g. how to not print random stuff to STDOUT while running
tests and how to stub in responses from a fake STDIN.

Tonkpils: Just as a side step for this, I haven't had too much time to look into what's going on but I noticed we're using a interaction package, this may be worth looking into: https://github.com/AlecAivazis/survey
kytrinyx: AlecAivazis/survey looks very nice! I will take it for a spin and see what it feels like.
kytrinyx: Another one worth checking out: https://github.com/c-bata/go-prompt
kytrinyx: My brain is in overload lately, so I'm going to go ahead and push forward with what I have, and then we can swap out the interaction part to make it more elegant once we've had a chance to test out some other libraries.

This should get us to "good enough to beta test" at least.
nywilken: All good here; merging.
robphoenix: > we can swap out the interaction part to make it more elegant once we've had a chance to test out some other libraries.

This looks good for now, and we can easily jazz it up later once the dust has settled a bit on a working version of the CLI. The `Survey` package looks quite good for this, whereas `go-prompt` looks interesting more for `REPL` type stuff. It's based off a Python package that's been used for some interesting projects like an improved Python REPL, rather than simpler Q&A prompts. Though at a future date it might be interesting to use for some kind of interactive stats command for users.
kytrinyx: > do you want to include all the text?

Good point @nywilken, I'll clean that up and merge this.

Patch:
diff --git a/cmd/open.go b/cmd/open.go
index f23a49d..c1b0cff 100644
--- a/cmd/open.go
+++ b/cmd/open.go
@@ -1,11 +1,9 @@
 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"
@@ -51,10 +49,7 @@ the solution you want to see on the website.
 			solutions = mine
 		}
 
-		selection := comms.NewSelection()
-		for _, solution := range solutions {
-			selection.Items = append(selection.Items, solution)
-		}
+		sx := workspace.Solutions(solutions)
 		for {
 			prompt := `
 We found more than one. Which one did you mean?
@@ -62,17 +57,13 @@ Type the number of the one you want to select.
 
 %s
 > `
-			option, err := selection.Pick(prompt)
+			s, err := sx.Pick(prompt)
 			if err != nil {
 				fmt.Println(err)
 				continue
 			}
-			solution, ok := option.(*workspace.Solution)
-			if ok {
-				browser.Open(solution.URL)
-				break
-			}
-			BailOnError(errors.New("should never happen"))
+			browser.Open(s.URL)
+			break
 		}
 
 	},
diff --git a/comms/question.go b/comms/question.go
new file mode 100644
index 0000000..494045b
--- /dev/null
+++ b/comms/question.go
@@ -0,0 +1,36 @@
+package comms
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"strings"
+)
+
+// Question provides an interactive session.
+type Question struct {
+	Reader       io.Reader
+	Writer       io.Writer
+	Prompt       string
+	DefaultValue string
+}
+
+// Read reads the user's input.
+func (q Question) Read(r io.Reader) (string, error) {
+	reader := bufio.NewReader(r)
+	s, err := reader.ReadString('\n')
+	if err != nil {
+		return "", err
+	}
+	s = strings.Trim(s, "\n")
+	if s == "" {
+		return q.DefaultValue, nil
+	}
+	return s, nil
+}
+
+// Ask displays the prompt, then records the response.
+func (q *Question) Ask() (string, error) {
+	fmt.Fprintf(q.Writer, q.Prompt)
+	return q.Read(q.Reader)
+}
diff --git a/comms/selection.go b/comms/selection.go
deleted file mode 100644
index 5d76086..0000000
--- a/comms/selection.go
+++ /dev/null
@@ -1,78 +0,0 @@
-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 20228ab..b285d0f 100644
--- a/workspace/solutions.go
+++ b/workspace/solutions.go
@@ -1,5 +1,15 @@
 package workspace
 
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strconv"
+	"strings"
+)
+
 // Solutions is a collection of solutions to interactively choose from.
 type Solutions []*Solution
 
@@ -16,3 +26,54 @@ 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/question_test.go b/comms/question_test.go
new file mode 100644
index 0000000..f7721ae
--- /dev/null
+++ b/comms/question_test.go
@@ -0,0 +1,33 @@
+package comms
+
+import (
+	"io/ioutil"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestQuestion(t *testing.T) {
+	tests := []struct {
+		desc     string
+		given    string
+		fallback string
+		expected string
+	}{
+		{"records interactive response", "hello\n", "", "hello"},
+		{"responds with default if response is empty", "\n", "Fine.", "Fine."},
+	}
+	for _, test := range tests {
+		q := &Question{
+			Reader:       strings.NewReader(test.given),
+			Writer:       ioutil.Discard,
+			Prompt:       "Say something: ",
+			DefaultValue: test.fallback,
+		}
+
+		answer, err := q.Ask()
+		assert.NoError(t, err)
+		assert.Equal(t, answer, test.expected, test.desc)
+	}
+}
diff --git a/comms/selection_test.go b/comms/selection_test.go
deleted file mode 100644
index ad5bcd0..0000000
--- a/comms/selection_test.go
+++ /dev/null
@@ -1,126 +0,0 @@
-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 65fdf6a..400d7ff 100644
--- a/workspace/solutions_test.go
+++ b/workspace/solutions_test.go
@@ -34,3 +34,32 @@ 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)
+}