Handle limited capacity history and the order, truncate history file as needed

Makefile

@@ -3,7 +3,7 @@ all: lint test demo
 GO_BUILD_TAGS:=no_net,no_json,no_pprof
 
 demo:
-	go run  -tags $(GO_BUILD_TAGS) ./example/ -loglevel debug
+	go run  -tags $(GO_BUILD_TAGS) ./example/ -loglevel debug -only-valid
 
 tinygo-demo:
 	# No luck on mac https://github.com/tinygo-org/tinygo/issues/4395

README.md

@@ -1,3 +1,4 @@
+[![Go Reference](https://pkg.go.dev/badge/fortio.org/terminal.svg)](https://pkg.go.dev/fortio.org/terminal)
 # terminal
 
 Wrapper around [x/term](https://github.com/golang/term) for a "readline" like library for

example/main.go

@@ -54,18 +54,39 @@
 	return
 }
 
+func AddOrReplaceHistory(t *terminal.Terminal, replace bool, l string) {
+	// if in default auto mode, we don't manage history
+	// we also don't add empty commands. (at start of program)
+	if t.AutoHistory() || l == "" {
+		return
+	}
+	log.LogVf("Adding to history %q replace %t", l, replace)
+	if replace {
+		t.ReplaceLatest(l)
+	} else {
+		t.AddToHistory(l)
+	}
+}
+
 func Main() int {
 	// Pending https://github.com/golang/go/issues/68780
 	flagHistory := flag.String("history", "/tmp/terminal_history", "History `file` to use")
+	flagMaxHistory := flag.Int("max-history", 10, "Max number of history lines to keep")
+	flagOnlyValid := flag.Bool("only-valid", false, "Demonstrates filtering of history, only adding valid commands to it")
 	cli.Main()
 	t, err := terminal.Open()
 	if err != nil {
 		return log.FErrf("Error opening terminal: %v", err)
 	}
 	defer t.Close()
+	onlyValid := *flagOnlyValid
+	if onlyValid {
+		t.SetAutoHistory(false)
+	}
 	t.SetPrompt("Terminal demo> ")
 	t.LoggerSetup()
-	if err := t.SetHistoryFile(*flagHistory); err != nil {
+	t.NewHistory(*flagMaxHistory)
+	if err = t.SetHistoryFile(*flagHistory); err != nil {
 		// error already logged
 		return 1
 	}
@@ -74,8 +95,13 @@
 	fmt.Fprintf(t.Out, "Try 'after duration text...' to see text showing in the middle of edits after said duration\n")
 	fmt.Fprintf(t.Out, "Try <tab> for auto completion\n")
 	t.SetAutoCompleteCallback(autoCompleteCallback)
+	previousCommandWasValid := true // won't be used because `line` is empty at start
+	isValidCommand := true
+	var cmd string
 	for {
-		l, err := t.ReadLine()
+		// Replace unless the previous command was valid.
+		AddOrReplaceHistory(t, !previousCommandWasValid, cmd)
+		cmd, err = t.ReadLine()
 		switch {
 		case err == nil:
 			// no error is good, nothing in this switch.
@@ -85,14 +111,18 @@
 		default:
 			return log.FErrf("Error reading line: %v", err)
 		}
-		log.Infof("Read line got: %q", l)
+		log.Infof("Read line got: %q", cmd)
+		// Save previous command validity to know whether this one should replace it in history or not.
+		previousCommandWasValid = isValidCommand
+		isValidCommand = false // not valid unless proven otherwise (reaches the end validations etc)
 		switch {
-		case l == exitCmd:
+		case cmd == exitCmd:
 			return 0
-		case l == helpCmd:
+		case cmd == helpCmd:
 			fmt.Fprintf(t.Out, "Available commands: %v\n", commands)
-		case strings.HasPrefix(l, afterCmd):
-			parts := strings.SplitN(l, " ", 3)
+			isValidCommand = true
+		case strings.HasPrefix(cmd, afterCmd):
+			parts := strings.SplitN(cmd, " ", 3)
 			if len(parts) < 3 {
 				fmt.Fprintf(t.Out, "Usage: %s <duration> <text...>\n", afterCmd)
 				continue
@@ -107,10 +137,15 @@
 				time.Sleep(dur)
 				fmt.Fprintf(t.Out, "%s\n", parts[2])
 			}()
-		case strings.HasPrefix(l, promptCmd):
-			t.SetPrompt(l[len(promptCmd):])
+			isValidCommand = true
+		case strings.HasPrefix(cmd, promptCmd):
+			if onlyValid {
+				t.AddToHistory(cmd)
+			}
+			t.SetPrompt(cmd[len(promptCmd):])
+			isValidCommand = true
 		default:
-			fmt.Fprintf(t.Out, "Unknown command %q\n", l)
+			fmt.Fprintf(t.Out, "Unknown command %q\n", cmd)
 		}
 	}
 }

go.mod

@@ -5,9 +5,11 @@ go 1.22.6
 require (
 	fortio.org/cli v1.8.0
 	fortio.org/log v1.16.0
-	fortio.org/term v0.23.0-fortio-1
+	fortio.org/term v0.23.0-fortio-4
 )
 
+// replace fortio.org/term => ../term
+
 require (
 	fortio.org/struct2env v0.4.1 // indirect
 	fortio.org/version v1.0.4 // indirect

go.sum

@@ -4,17 +4,13 @@ fortio.org/log v1.16.0 h1:GhU8/9NkYZmEIzvTN/DTMedDAStLJraWUUVUA2EbNDc=
 fortio.org/log v1.16.0/go.mod h1:t58Spg9njjymvRioh5F6qKGSupEsnMjXLGWIS1i3khE=
 fortio.org/struct2env v0.4.1 h1:rJludAMO5eBvpWplWEQNqoVDFZr4RWMQX7RUapgZyc0=
 fortio.org/struct2env v0.4.1/go.mod h1:lENUe70UwA1zDUCX+8AsO663QCFqYaprk5lnPhjD410=
-fortio.org/term v0.23.0-fortio-1 h1:bBfsi9yGE5aI5FDnBxluIc7+7Gc2vWdgdckJeDYRGsc=
-fortio.org/term v0.23.0-fortio-1/go.mod h1:7buBfn81wEJUGWiVjFNiUE/vxWs5FdM9c7PyZpZRS30=
+fortio.org/term v0.23.0-fortio-4 h1:vLK1/UYCLW1kznWcZb0Zcgmb6rC3Kw0mDhDCoI63boY=
+fortio.org/term v0.23.0-fortio-4/go.mod h1:7buBfn81wEJUGWiVjFNiUE/vxWs5FdM9c7PyZpZRS30=
 fortio.org/version v1.0.4 h1:FWUMpJ+hVTNc4RhvvOJzb0xesrlRmG/a+D6bjbQ4+5U=
 fortio.org/version v1.0.4/go.mod h1:2JQp9Ax+tm6QKiGuzR5nJY63kFeANcgrZ0osoQFDVm0=
 github.com/kortschak/goroutine v1.1.2 h1:lhllcCuERxMIK5cYr8yohZZScL1na+JM5JYPRclWjck=
 github.com/kortschak/goroutine v1.1.2/go.mod h1:zKpXs1FWN/6mXasDQzfl7g0LrGFIOiA6cLs9eXKyaMY=
-golang.org/x/crypto/x509roots/fallback v0.0.0-20240626151235-a6a393ffd658 h1:i7K6wQLN/0oxF7FT3tKkfMCstxoT4VGG36YIB9ZKLzI=
-golang.org/x/crypto/x509roots/fallback v0.0.0-20240626151235-a6a393ffd658/go.mod h1:kNa9WdvYnzFwC79zRpLRMJbdEFlhyM5RPFBBZp/wWH8=
 golang.org/x/crypto/x509roots/fallback v0.0.0-20240806160748-b2d3a6a4b4d3 h1:oWb21rU9Q9XrRwXLB7jHc1rbp6EiiimZZv5MLxpu4T0=
 golang.org/x/crypto/x509roots/fallback v0.0.0-20240806160748-b2d3a6a4b4d3/go.mod h1:kNa9WdvYnzFwC79zRpLRMJbdEFlhyM5RPFBBZp/wWH8=
-golang.org/x/sys v0.23.0 h1:YfKFowiIMvtgl1UERQoTPPToxltDeZfbj4H7dVUCwmM=
-golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.24.0 h1:Twjiwq9dn6R1fQcyiK+wQyHWfaz/BJB+YIpzU/Cv3Xg=
 golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=

terminal.go

@@ -6,6 +6,8 @@
 	"errors"
 	"io"
 	"os"
+	"slices"
+	"strconv"
 
 	"fortio.org/log"
 	"fortio.org/term"
@@ -17,6 +19,8 @@
 	term        *term.Terminal
 	Out         io.Writer
 	historyFile string
+	capacity    int
+	autoHistory bool
 }
 
 // Open opens stdin as a terminal, do `defer terminal.Close()`
@@ -41,6 +45,7 @@
 		return nil, err
 	}
 	t.term.SetBracketedPasteMode(true) // Seems useful to have it on by default.
+	t.capacity = term.DefaultHistoryEntries
 	return t, nil
 }
 
@@ -62,6 +67,10 @@
 		log.Infof("No history file specified")
 		return nil
 	}
+	if t.capacity <= 0 {
+		log.Infof("No history capacity set, ignoring history file %s", f)
+		return nil
+	}
 	if !t.IsTerminal() {
 		log.Infof("Not a terminal, not setting history file")
 		return nil
@@ -72,10 +81,16 @@
 		t.historyFile = "" // so we don't try to save during defer'ed close if we can't read
 		return err
 	}
-	for _, e := range entries {
+	start := 0
+	if len(entries) > t.capacity {
+		log.Infof("History file %s has more than %d entries, truncating.", f, t.capacity)
+		start = len(entries) - t.capacity
+	} else {
+		log.Infof("Loaded %d history entries from %s", len(entries), f)
+	}
+	for _, e := range entries[start:] {
 		t.term.AddToHistory(e)
 	}
-	log.Infof("Loaded %d history entries from %s", len(entries), f)
 	return nil
 }
 
@@ -92,10 +107,32 @@
 }
 
 // NewHistory creates/resets the history to a new one with the given capacity.
+// need + 1 to fit "pending" command.
 func (t *Terminal) NewHistory(capacity int) {
+	if capacity < 0 {
+		log.Errf("Invalid history capacity %d, ignoring", capacity)
+		return
+	}
+	t.capacity = capacity
 	t.term.NewHistory(capacity)
 }
 
+// SetAutoHistory enables/disables auto history (default is enabled).
+func (t *Terminal) SetAutoHistory(enabled bool) {
+	t.autoHistory = enabled
+	t.term.AutoHistory(enabled)
+}
+
+// AutoHistory returns the current auto history setting.
+func (t *Terminal) AutoHistory() bool {
+	return t.autoHistory
+}
+
+// ReplaceLatest replaces the current history with the given commands, returns the previous value.
+func (t *Terminal) ReplaceLatest(command string) string {
+	return t.term.ReplaceLatest(command)
+}
+
 func readOrCreateHistory(f string) ([]string, error) {
 	// open file or create it
 	h, err := os.OpenFile(f, os.O_RDWR|os.O_CREATE, 0o600)
@@ -108,7 +145,14 @@
 	var lines []string
 	scanner := bufio.NewScanner(h)
 	for scanner.Scan() {
-		lines = append(lines, scanner.Text())
+		// unquote to get the actual command
+		rl := scanner.Text()
+		l, err := strconv.Unquote(rl)
+		if err != nil {
+			log.Errf("Error unquoting history file %s for %q: %v", f, rl, err)
+			return nil, err
+		}
+		lines = append(lines, l)
 	}
 	if err := scanner.Err(); err != nil {
 		log.Errf("Error reading history file %s: %v", f, err)
@@ -118,20 +162,16 @@
 }
 
 func saveHistory(f string, h []string) {
-	if f == "" {
-		log.Infof("No history file specified")
-		return
-	}
 	// open file or create it
-	hf, err := os.OpenFile(f, os.O_RDWR|os.O_CREATE, 0o600)
+	hf, err := os.OpenFile(f, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0o600)
 	if err != nil {
 		log.Errf("Error opening history file %s: %v", f, err)
 		return
 	}
 	defer hf.Close()
 	// write lines separated by \n
 	for _, l := range h {
-		_, err := hf.WriteString(l + "\n")
+		_, err := hf.WriteString(strconv.Quote(l) + "\n")
 		if err != nil {
 			log.Errf("Error writing history file %s: %v", f, err)
 			return
@@ -143,15 +183,26 @@
 	if t.oldState == nil {
 		return nil
 	}
+	// To avoid prompt being repeated on the last line (shouldn't be necessary but... is
+	// consider fixing in term instead)
+	t.term.SetPrompt("") // will still reprint the last command on ^C in middle of typing.
 	err := term.Restore(t.fd, t.oldState)
 	t.oldState = nil
 	t.Out = os.Stderr
 	// saving history if any
-	if t.historyFile != "" {
-		h := t.term.History()
-		log.Infof("Saving history (%d commands) to %s", len(h), t.historyFile)
-		saveHistory(t.historyFile, h)
+	if t.historyFile == "" || t.capacity <= 0 {
+		log.Debugf("No history file %q or capacity %d, not saving history", t.historyFile, t.capacity)
+		return nil
+	}
+	h := t.term.History()
+	log.LogVf("got history %v", h)
+	slices.Reverse(h)
+	extra := len(h) - t.capacity
+	if extra > 0 {
+		h = h[extra:] // truncate to max capacity otherwise extra ones will get out of order
 	}
+	log.Infof("Saving history (%d commands) to %s", len(h), t.historyFile)
+	saveHistory(t.historyFile, h)
 	return err
 }