From da729e5b386ca0fd32344dcc1fd63d14c0bb39ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= <peterke@gmail.com>
Date: Mon, 30 May 2016 17:30:17 +0300
Subject: [PATCH] cmd/geth, console: fix reviewer issues

---
 cmd/geth/accountcmd.go  |  4 +-
 cmd/geth/chaincmd.go    |  2 +-
 cmd/geth/main.go        |  2 +-
 console/console.go      | 23 ++++++++----
 console/console_test.go |  3 +-
 console/prompter.go     | 83 +++++++++++++++++++++++------------------
 6 files changed, 68 insertions(+), 49 deletions(-)

diff --git a/cmd/geth/accountcmd.go b/cmd/geth/accountcmd.go
index a9cee20ee..0f9d95c2c 100644
--- a/cmd/geth/accountcmd.go
+++ b/cmd/geth/accountcmd.go
@@ -216,12 +216,12 @@ func getPassPhrase(prompt string, confirmation bool, i int, passwords []string)
 	if prompt != "" {
 		fmt.Println(prompt)
 	}
-	password, err := console.TerminalPrompter.PromptPassword("Passphrase: ")
+	password, err := console.Stdin.PromptPassword("Passphrase: ")
 	if err != nil {
 		utils.Fatalf("Failed to read passphrase: %v", err)
 	}
 	if confirmation {
-		confirm, err := console.TerminalPrompter.PromptPassword("Repeat passphrase: ")
+		confirm, err := console.Stdin.PromptPassword("Repeat passphrase: ")
 		if err != nil {
 			utils.Fatalf("Failed to read passphrase confirmation: %v", err)
 		}
diff --git a/cmd/geth/chaincmd.go b/cmd/geth/chaincmd.go
index 457dbcfff..4f47de5d7 100644
--- a/cmd/geth/chaincmd.go
+++ b/cmd/geth/chaincmd.go
@@ -117,7 +117,7 @@ func exportChain(ctx *cli.Context) {
 }
 
 func removeDB(ctx *cli.Context) {
-	confirm, err := console.TerminalPrompter.PromptConfirm("Remove local database?")
+	confirm, err := console.Stdin.PromptConfirm("Remove local database?")
 	if err != nil {
 		utils.Fatalf("%v", err)
 	}
diff --git a/cmd/geth/main.go b/cmd/geth/main.go
index cdbda53e8..5ff1a7368 100644
--- a/cmd/geth/main.go
+++ b/cmd/geth/main.go
@@ -236,7 +236,7 @@ participating.
 	app.After = func(ctx *cli.Context) error {
 		logger.Flush()
 		debug.Exit()
-		console.TerminalPrompter.Close() // Resets terminal mode.
+		console.Stdin.Close() // Resets terminal mode.
 		return nil
 	}
 }
diff --git a/console/console.go b/console/console.go
index d10353093..a19b267bc 100644
--- a/console/console.go
+++ b/console/console.go
@@ -74,7 +74,7 @@ type Console struct {
 func New(config Config) (*Console, error) {
 	// Handle unset config values gracefully
 	if config.Prompter == nil {
-		config.Prompter = TerminalPrompter
+		config.Prompter = Stdin
 	}
 	if config.Prompt == "" {
 		config.Prompt = DefaultPrompt
@@ -192,9 +192,10 @@ func (c *Console) init(preload []string) error {
 	// Configure the console's input prompter for scrollback and tab completion
 	if c.prompter != nil {
 		if content, err := ioutil.ReadFile(c.histPath); err != nil {
-			c.prompter.SetScrollHistory(nil)
+			c.prompter.SetHistory(nil)
 		} else {
-			c.prompter.SetScrollHistory(strings.Split(string(content), "\n"))
+			c.history = strings.Split(string(content), "\n")
+			c.prompter.SetHistory(c.history)
 		}
 		c.prompter.SetWordCompleter(c.AutoCompleteInput)
 	}
@@ -322,7 +323,7 @@ func (c *Console) Interactive() {
 
 		case line, ok := <-scheduler:
 			// User input was returned by the prompter, handle special cases
-			if !ok || (indents <= 0 && exit.MatchString(input)) {
+			if !ok || (indents <= 0 && exit.MatchString(line)) {
 				return
 			}
 			if onlyWhitespace.MatchString(line) {
@@ -339,8 +340,13 @@ func (c *Console) Interactive() {
 			}
 			// If all the needed lines are present, save the command and run
 			if indents <= 0 {
-				if len(input) != 0 && input[0] != ' ' && !passwordRegexp.MatchString(input) {
-					c.history = append(c.history, input[:len(input)-1])
+				if len(input) > 0 && input[0] != ' ' && !passwordRegexp.MatchString(input) {
+					if command := strings.TrimSpace(input); len(c.history) == 0 || command != c.history[len(c.history)-1] {
+						c.history = append(c.history, command)
+						if c.prompter != nil {
+							c.prompter.AppendHistory(command)
+						}
+					}
 				}
 				c.Evaluate(input)
 				input = ""
@@ -356,7 +362,10 @@ func (c *Console) Execute(path string) error {
 
 // Stop cleans up the console and terminates the runtime envorinment.
 func (c *Console) Stop(graceful bool) error {
-	if err := ioutil.WriteFile(c.histPath, []byte(strings.Join(c.history, "\n")), os.ModePerm); err != nil {
+	if err := ioutil.WriteFile(c.histPath, []byte(strings.Join(c.history, "\n")), 0600); err != nil {
+		return err
+	}
+	if err := os.Chmod(c.histPath, 0600); err != nil { // Force 0600, even if it was different previously
 		return err
 	}
 	c.jsre.Stop(graceful)
diff --git a/console/console_test.go b/console/console_test.go
index 72d3a2df6..911087824 100644
--- a/console/console_test.go
+++ b/console/console_test.go
@@ -68,7 +68,8 @@ func (p *hookedPrompter) PromptPassword(prompt string) (string, error) {
 func (p *hookedPrompter) PromptConfirm(prompt string) (bool, error) {
 	return false, errors.New("not implemented")
 }
-func (p *hookedPrompter) SetScrollHistory(history []string)        {}
+func (p *hookedPrompter) SetHistory(history []string)              {}
+func (p *hookedPrompter) AppendHistory(command string)             {}
 func (p *hookedPrompter) SetWordCompleter(completer WordCompleter) {}
 
 // tester is a console test environment for the console tests to operate on.
diff --git a/console/prompter.go b/console/prompter.go
index 5039e8b1c..0e4a8a53e 100644
--- a/console/prompter.go
+++ b/console/prompter.go
@@ -23,10 +23,9 @@ import (
 	"github.com/peterh/liner"
 )
 
-// TerminalPrompter holds the stdin line reader (also using stdout for printing
-// prompts). Only this reader may be used for input because it keeps an internal
-// buffer.
-var TerminalPrompter = newTerminalPrompter()
+// Stdin holds the stdin line reader (also using stdout for printing prompts).
+// Only this reader may be used for input because it keeps an internal buffer.
+var Stdin = newTerminalPrompter()
 
 // UserPrompter defines the methods needed by the console to promt the user for
 // various types of inputs.
@@ -44,9 +43,13 @@ type UserPrompter interface {
 	// choice to be made, returning that choice.
 	PromptConfirm(prompt string) (bool, error)
 
-	// SetScrollHistory sets the the input scrollback history that the prompter will
-	// allow the user to scoll back to.
-	SetScrollHistory(history []string)
+	// SetHistory sets the the input scrollback history that the prompter will allow
+	// the user to scoll back to.
+	SetHistory(history []string)
+
+	// AppendHistory appends an entry to the scrollback history. It should be called
+	// if and only if the prompt to append was a valid command.
+	AppendHistory(command string)
 
 	// SetWordCompleter sets the completion function that the prompter will call to
 	// fetch completion candidates when the user presses tab.
@@ -74,34 +77,34 @@ type terminalPrompter struct {
 // newTerminalPrompter creates a liner based user input prompter working off the
 // standard input and output streams.
 func newTerminalPrompter() *terminalPrompter {
-	r := new(terminalPrompter)
+	p := new(terminalPrompter)
 	// Get the original mode before calling NewLiner.
 	// This is usually regular "cooked" mode where characters echo.
 	normalMode, _ := liner.TerminalMode()
 	// Turn on liner. It switches to raw mode.
-	r.State = liner.NewLiner()
+	p.State = liner.NewLiner()
 	rawMode, err := liner.TerminalMode()
 	if err != nil || !liner.TerminalSupported() {
-		r.supported = false
+		p.supported = false
 	} else {
-		r.supported = true
-		r.normalMode = normalMode
-		r.rawMode = rawMode
+		p.supported = true
+		p.normalMode = normalMode
+		p.rawMode = rawMode
 		// Switch back to normal mode while we're not prompting.
 		normalMode.ApplyMode()
 	}
-	r.SetCtrlCAborts(true)
-	r.SetTabCompletionStyle(liner.TabPrints)
+	p.SetCtrlCAborts(true)
+	p.SetTabCompletionStyle(liner.TabPrints)
 
-	return r
+	return p
 }
 
 // PromptInput displays the given prompt to the user and requests some textual
 // data to be entered, returning the input of the user.
-func (r *terminalPrompter) PromptInput(prompt string) (string, error) {
-	if r.supported {
-		r.rawMode.ApplyMode()
-		defer r.normalMode.ApplyMode()
+func (p *terminalPrompter) PromptInput(prompt string) (string, error) {
+	if p.supported {
+		p.rawMode.ApplyMode()
+		defer p.normalMode.ApplyMode()
 	} else {
 		// liner tries to be smart about printing the prompt
 		// and doesn't print anything if input is redirected.
@@ -110,47 +113,53 @@ func (r *terminalPrompter) PromptInput(prompt string) (string, error) {
 		prompt = ""
 		defer fmt.Println()
 	}
-	return r.State.Prompt(prompt)
+	return p.State.Prompt(prompt)
 }
 
 // PromptPassword displays the given prompt to the user and requests some textual
 // data to be entered, but one which must not be echoed out into the terminal.
 // The method returns the input provided by the user.
-func (r *terminalPrompter) PromptPassword(prompt string) (passwd string, err error) {
-	if r.supported {
-		r.rawMode.ApplyMode()
-		defer r.normalMode.ApplyMode()
-		return r.State.PasswordPrompt(prompt)
+func (p *terminalPrompter) PromptPassword(prompt string) (passwd string, err error) {
+	if p.supported {
+		p.rawMode.ApplyMode()
+		defer p.normalMode.ApplyMode()
+		return p.State.PasswordPrompt(prompt)
 	}
-	if !r.warned {
+	if !p.warned {
 		fmt.Println("!! Unsupported terminal, password will be echoed.")
-		r.warned = true
+		p.warned = true
 	}
 	// Just as in Prompt, handle printing the prompt here instead of relying on liner.
 	fmt.Print(prompt)
-	passwd, err = r.State.Prompt("")
+	passwd, err = p.State.Prompt("")
 	fmt.Println()
 	return passwd, err
 }
 
 // PromptConfirm displays the given prompt to the user and requests a boolean
 // choice to be made, returning that choice.
-func (r *terminalPrompter) PromptConfirm(prompt string) (bool, error) {
-	input, err := r.Prompt(prompt + " [y/N] ")
+func (p *terminalPrompter) PromptConfirm(prompt string) (bool, error) {
+	input, err := p.Prompt(prompt + " [y/N] ")
 	if len(input) > 0 && strings.ToUpper(input[:1]) == "Y" {
 		return true, nil
 	}
 	return false, err
 }
 
-// SetScrollHistory sets the the input scrollback history that the prompter will
-// allow the user to scoll back to.
-func (r *terminalPrompter) SetScrollHistory(history []string) {
-	r.State.ReadHistory(strings.NewReader(strings.Join(history, "\n")))
+// SetHistory sets the the input scrollback history that the prompter will allow
+// the user to scoll back to.
+func (p *terminalPrompter) SetHistory(history []string) {
+	p.State.ReadHistory(strings.NewReader(strings.Join(history, "\n")))
+}
+
+// AppendHistory appends an entry to the scrollback history. It should be called
+// if and only if the prompt to append was a valid command.
+func (p *terminalPrompter) AppendHistory(command string) {
+	p.State.AppendHistory(command)
 }
 
 // SetWordCompleter sets the completion function that the prompter will call to
 // fetch completion candidates when the user presses tab.
-func (r *terminalPrompter) SetWordCompleter(completer WordCompleter) {
-	r.State.SetWordCompleter(liner.WordCompleter(completer))
+func (p *terminalPrompter) SetWordCompleter(completer WordCompleter) {
+	p.State.SetWordCompleter(liner.WordCompleter(completer))
 }
-- 
GitLab