Terminal bell is ringing when try to select item using up-down arrow(↑↓) keys

[hallazzang]

I'm on macOS High Sierra, using iTerm2 3.1.5, zsh 5.3, Go 1.9.2 darwin/amd64 and the latest version of promptui.

While taking a look at #41, I found that my terminal alerts me with a ringing bell when I try to select item using up-down arrow(↑↓) keys, even I'm not on the top-most or the bottom-most.

It is happening on different terminals, for instance macOS' default terminal 'Terminal.app' and different shells, like sh, bash or csh.

It's not that important problem at all, but users might get annoyed by this.

[hallazzang]

Things get worse when I turn on the 'Vim Mode', by passing IsVimMode: true to Select.

At the initial state, like when I first run the program with go run main.go, j and k keys are working well without any terminal bell but after pressing one of the arrow keys(←→↑↓), terminal begins to ringing if I press any key--even j, k keys.

I think this is readline-related issue, I'll try to figure out what is wrong by myself.

[hallazzang]

Ok, finally I found the answer. The terminal bell was coming from chzyer/readline/operation.go:265-278. After commenting out the o.t.Bell() lines, annoying bell is gone. But without hard-patching the code, there seems no other solution for now. It is desired choice to reporting terminal bell when user tries to access invalid history for readline, but it does not fit for promptui's case(doesn't using readline's history feature at all by setting readline.Config.HistoryLimit to -1).

I think it'd be good to slightly modify the original readline source and put it in the Go's vendor directory or elsewhere and then put some comment there.

Or... what about reinventing the wheel--write own key handler just for promptui's needs. It could fix #33, too!

ADD: Commenting out chzyer/readline/vim.go:149 would remove the bell in the 'Vim Mode' I've mentioned earlier.

[louisbranch]

Thanks for the detective work!

We could propose a patch to readline to make the bell optional. I rather not forking / come up with our own implementation if we can avoid. If we can find a solution for Windows, readline users would also benefit from the fix.

[hallazzang]

Ah, optional bell that would be a great solution too. BTW, I was planning to build a relatively small packge for key handling in Go since it looks there are not. The package will not be strongly related to prompui, but I'll let you know if my package works well and can be adopted in promptui smoothly--without breaking the design goal of this package.

[tisba]

Hey everyone. Has there been any update regarding this issue? The bell is really annoying :-/

[maraino]

A hacky solution for this is to override readline.Stdout with your own implementation and skip the bell. Here I'm also using os.Stderr instead of os.Stdout to be able to pipe the information in the standard output to other programs:

type stderr struct{}

func (s *stderr) Write(b []byte) (int, error) {
	if len(b) == 1 && b[0] == 7 {
		return 0, nil
	}
	return os.Stderr.Write(b)
}

func (s *stderr) Close() error {
	return os.Stderr.Close()
}

func init() {
	readline.Stdout = &stderr{}
}

[maraino]

@luizbranco: my hack above works like a charm, but it would be great to be able to be able to set custom Stdin, Stdout, and Stderr in prompts and selects, if they are not set, then use the defaults. That will probably allow promptui to switch implementations if you need to. I'm willing to provide a PR if you think that's the right approach.

[danlsgiga]

@maraino where are you putting the proposed code above?

[maraino]

@danlsgiga it really does not matter, just in a package that gets initialized when the program starts.

In my case, I have it on a wrapper of the promptui methods that I use, this wrapper adds some extra functionality that I need and fixes the display of prompts when the program receives data on STDIN. https://github.com/smallstep/cli/blob/master/ui/ui.go#L15-L36

[Issif]

any update about this issue? thanks

[mroth]

re: @maraino

my hack above works like a charm, but it would be great to be able to be able to set custom Stdin, Stdout, and Stderr in prompts and selects, if they are not set, then use the defaults.

This appears to be possible-(ish) now. I am currently using the following very slightly modified version of your sample code as such:

// bellSkipper implements an io.WriteCloser that skips the terminal bell
// character (ASCII code 7), and writes the rest to os.Stderr. It is used to
// replace readline.Stdout, that is the package used by promptui to display the
// prompts.
//
// This is a workaround for the bell issue documented in
// https://github.com/manifoldco/promptui/issues/49.
type bellSkipper struct{}

// Write implements an io.WriterCloser over os.Stderr, but it skips the terminal
// bell character.
func (bs *bellSkipper) Write(b []byte) (int, error) {
	const charBell = 7 // c.f. readline.CharBell
	if len(b) == 1 && b[0] == charBell {
		return 0, nil
	}
	return os.Stderr.Write(b)
}

// Close implements an io.WriterCloser over os.Stderr.
func (bs *bellSkipper) Close() error {
	return os.Stderr.Close()
}

I then pass it at struct init such as:

prompt := promptui.Select{
	// ...snip...
	Stdout: &bellSkipper{},
}

This slightly modified method allows me to avoid having to import chyzer/readline in my own package, and avoids usage of a global init() function.

Additionally, I believe it should now be possible to PR a modification to promptui.Select and promptui.Prompt to fall back to using this implementation during Run() when p.Stdout is nil, fixing this issue by default for most downsteam users of promptui, but avoiding modifying readline globally. (Before working on a PR for that, am I perhaps missing something obvious?)

[marcauberer]

@mroth: The workaround above does not work anymore. The outcome is something like this: Any suggestion on this?

[jesselang]

@mroth: The workaround above does not work anymore. Any suggestion on this?

It appears the terminal you are using does not support ANSI escape codes. This appears to be an altogether different problem than what is being discussed in this issue.

[levanidev]

@mroth: The workaround above does not work anymore. Any suggestion on this?

It appears the terminal you are using does not support ANSI escape codes. This appears to be an altogether different problem than what is being discussed in this issue.

That's actually not the case. After changing to bellSkipper as @maraino implemented it. I get same results as @marcauberer in powershell

[maraino]

@Itshardtopickanusername Try to use windows.SetConsoleMode() something like this:

func init() {
	var inMode, outMode uint32
	if err := windows.GetConsoleMode(windows.Stdin, &inMode); err == nil {
		inMode |= windows.ENABLE_VIRTUAL_TERMINAL_INPUT
		if err := windows.SetConsoleMode(windows.Stdin, inMode); err != nil {
			fmt.Fprintf(os.Stderr, "Failed to set console mode: %v", err)
		}
	}
	if err := windows.GetConsoleMode(windows.Stdout, &outMode); err == nil {
		outMode |= windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING
		if err := windows.SetConsoleMode(windows.Stdout, outMode); err != nil {
			fmt.Fprintf(os.Stderr, "Failed to set console mode: %v", err)
		}
	}
}

https://docs.microsoft.com/en-us/windows/console/setconsolemode

[NachE]

For those looking for a less intrusive and Windows compatible workaround. Inspired by @maraino and @mroth solution.

In my case, importing the chyzer/readline library is indifferent to me since it is already imported by promptui anyway.

import "github.com/chzyer/readline"

type noBellStdout struct{}

func (n *noBellStdout) Write(p []byte) (int, error) {
	if len(p) == 1 && p[0] == readline.CharBell {
		return 0, nil
	}
	return readline.Stdout.Write(p)
}

func (n *noBellStdout) Close() error {
	return readline.Stdout.Close()
}

var NoBellStdout = &noBellStdout{}

This is just a wrapper of the original readline Stdout, which in turn is different depending on the OS.

prompt := promptui.Select{
	// ...
	Stdout: NoBellStdout,
}