x/term: support pluggable history

[ldemailly]

Proposal Details

--- Accepted proposal:

// A History provides a (possibly bounded) queue of input lines read by [ReadLine].
type History interface {
	// Add adds a new, most recent entry to the history.
	// It is allowed to drop any entry, including
	// the entry being added (e.g.,, if it's a whitespace-only entry),
	// the least-recent entry (e.g., to keep the history bounded),
	// or any other entry.
	Add(entry string)

	// Len returns the number of entries in the history.
	Len() int

	// At returns an entry from the history.
	// Index 0 is the most-recently added entry and
	// index Len()-1 is the least-recently added entry.
	// If index is < 0 or >= Len(), it panics.
	At(index int) string
}

type Terminal struct {
        ...

        // History records and retrieves lines of input read by [ReadLine].
        //
        // It is not safe to call ReadLine concurrently with any methods on History.
        // 
        // [NewTerminal] sets this to an implementation that records the last 100 lines of input.
        History History
}

copied from #68780 (comment)

--- Was "Interface" Proposal:

https://github.com/golang/term/pull/20/files

// The History interface provides a way to replace the default automatic
// 100 slots ringbuffer implementation.
type History interface {
	// Add will be called by Terminal to add a new line into the history.
	// An implementation may decide to not add every lines by ignoring calls
	// to this function (from Terminal.ReadLine) and to only add validated
	// entries out of band.
	Add(entry string)

	// At retrieves a line from history.
	// idx 0 should be the most recent entry.
	// return false when out of range.
	At(idx int) (string, bool)
}

// In terminal:

	// History allows to replace the default implementation of the history
	// which contains previously entered commands so that they can be
	// accessed with the up and down keys.
	// It's not safe to call ReadLine and methods on History concurrently.
	History History

---

And even earlier "direct" proposal for reference:

x/term is very nice, it includes a 100 slot history ring buffer, unfortunately entirely private

https://github.com/golang/term/blob/master/terminal.go#L89

it would be nice to let people

// History returns a slice of strings containing the history of entered commands so far.
func (t *Terminal) History() []string {

and

// AddToHistory populates history.
func (t *Terminal) AddToHistory(commands ...string) {

and resize

// NewHistory resets the history to one of a given capacity.
func (t *Terminal) NewHistory(capacity int) {

and allow replacement of last command in history (for instance to replace invalid by validated or canonical variant)

// ReplaceLatest replaces the most recent history entry with the given string.
// Enables to add invalid commands to the history for editing purpose and
// replace them with the corrected version. Returns the replaced entry.
func (t *Terminal) ReplaceLatest(entry string) string {

and control whether lines are automatically added to history (defaults remains true)

// AutoHistory sets whether lines are automatically added to the history
// before being returned by ReadLine() or not. Defaults to true.
func (t *Terminal) AutoHistory(onOff bool) {

if acceptable I'd be happy to make a PR edit: made a PR

[ldemailly]

Implementation golang/term#15 demo of use/benefit fortio/terminal@77eee7b

[gopherbot]

Change https://go.dev/cl/603957 mentions this issue: x/term: expose History() AddToHistory() and NewHistory() so users of the library can save and restore the history.

[earthboundkid]

I don't know if this is a good proposal or not, but if you add History() it should return iter.Seq[string].

[ldemailly]

I don't know if this is a good proposal or not, but if you add History() it should return iter.Seq[string].

Thanks for looking. If you look at the code you'll see it creates a copy while holding the lock. An iterator wouldn't be helpful as the copy (snapshot) is needed.

[vingarzan]

Hey Laurent!

I was about to push for review my patches for almost the same thing, hence would rather not waste everyone's time again and join you here.

I also did a terminal.PopHistory(), which I used when the user was just hitting Enter to clear the screen a bit. I think your terminal.ReplaceLatest() wouldn't cover this use case, of skipping the last command entirely from history, correct?

And what would you think is the best approach to achieve that? A modification to ReplaceLatest() (e.g. return a *string and nil means pop it), or the following that I already did in my patch?

// Pop the last element from the history.
func (s *stRingBuffer) Pop() (string, bool) {
	if s.size == 0 {
		return "", false
	}
	value := s.entries[s.head]
	s.head--
	if s.head < 0 {
		s.head += s.max
	}
	if s.size > 0 {
		s.size--
	}
	return value, true
}
func (t *Terminal) PopHistory() (string, bool) {
	t.lock.Lock()
	defer t.lock.Unlock()

	return t.history.Pop()
}

Or I guess also using AutoHistory(false) and then adding just the non-empty lines to history would work too...

Would you care to include this too, or you'd rather prefer that I'll push a PR after the current code is merged?

Cheers,
-Dragos

[ldemailly]

Thanks, yes I ended up mostly turning auto history off for real cases and adding depending on conditions, the on behavior is for backward compatibility with current behavior. I was using replace until I hit writing a "history" (and !nn repeat) which shifts the history by one making auto not useable, maybe for minimalism I should remove replace?

Edit: I wouldn't mind replacing Replace by Pop btw if that works better