Refactor confirmation prompt to separate pkg

[Stavrospanakakis]

This commit refactors the confirmation prompt to a separate pkg.

Signed-off-by: Stavros Panakakis stavrospanakakis@gmail.com

[marcosnils]

I'm thinking about the reason to have a tmpfile here. Why couldn't we just convert the answer to an io.Reader and since we're extracting this to a package, we can make the Confirm function use the reader as well instead of os.Stdin.

thoughts?

[Stavrospanakakis]

Do you mean something like this?

// prompt.go
package prompt
.
.
.
func Confirm(rd io.Reader, message string) error {
        fmt.Printf("\n%s [Y/n] ", message)
	reader := bufio.NewReader(rd)
        .
        .
}

// prune.go
package cmd
.
.
.
func newPruneCmd() *pruneCmd {
        .
        .
        .
        .
	if !root.opts.force {
                err := prompt.Confirm(os.Stdin, "The following paths will be removed. Continue?")
		if err != nil {
		        return err
		}
	}
      .
      .   

// prompt_test.go
func TestConfirm(t *testing.T) {
	t.Run("User confirms that wants to continue", func(t *testing.T) {
		answers := [][]byte{
			[]byte("Y\n"),
			[]byte("y\n"),
			[]byte("\n"),
		}

		for _, answer := range answers {
			rd := bytes.NewReader(answer)
			err := Confirm(rd, "Do you want to continue?")
			if err != nil {
				t.Fatal(err)
			}
		}
	})
}

[marcosnils]

sending os.Stdin to Confirm might be unnecessary. Why not just declaring the stdin at the package level and then making the test override it to a custom io.Reader?

something like:

--- main.go ---

var stdin io.Reader = os.Stdin

func Confirm(prompt string) error {
 // use stdin here
}

--- main_test.go ---

func TestConfirm (t *testing.T) {
  stdin = bytes.NewReader([]byte("whatever"))
  // test confirm here

}

thoughts?

[Stavrospanakakis]

Fixed :)

[marcosnils]

Loved it, thx ❤️

[marcosnils]

SGTM!

cc @sirlatrom @korpa @breml 👀 ?

[sirlatrom]

How about providing the io.Reader as a parameter to Confirm instead?

[marcosnils]

How about providing the io.Reader as a parameter to Confirm instead?

Check #132 (comment). I though it was easier to handle it separately 🙏

[Stavrospanakakis]

The branch had conflicts so I rebased :)