elvish icon indicating copy to clipboard operation
elvish copied to clipboard

listbox_window: integer divide by zero

Open rsteube opened this issue 1 year ago • 4 comments

What happened, and what did you expect to happen?

Seems there is still a divide by zero possible:

https://github.com/elves/elvish/blob/2e527f77fdc7206d4a9761cd68e1082e16d9e8a2/pkg/cli/tk/listbox_window.go#L128

Steps to reproduce:

  • resize window to an unreasonably small size: 7x3 (less width then column of currently selected what causes it?).
  • invoke completion
goroutine 1 [running]:
src.elv.sh/pkg/sys.DumpStack()
	/home/rsteube/Documents/development/github/elvish/pkg/sys/dumpstack.go:10 +0x65
src.elv.sh/pkg/shell.handlePanic()
	/home/rsteube/Documents/development/github/elvish/pkg/shell/interact.go:137 +0x4f
panic({0x9305a0?, 0xc97da0?})
	/usr/lib/go/src/runtime/panic.go:770 +0x132
src.elv.sh/pkg/cli/tk.getHorizontalWindow({{0xa47ad0, 0xc000948b10}, 0x0, 0x0, 0x0}, 0x0, 0x7, 0x0)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/tk/listbox_window.go:128 +0x226
src.elv.sh/pkg/cli/tk.(*listBox).renderHorizontal.func1(0xc0001600f8)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/tk/listbox.go:119 +0xdc
src.elv.sh/pkg/cli/tk.(*listBox).mutate(0xc000160090, 0xc00067f4f0)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/tk/listbox.go:421 +0x64
src.elv.sh/pkg/cli/tk.(*listBox).renderHorizontal(0xc000160090, 0x7, 0x0)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/tk/listbox.go:115 +0xcb
src.elv.sh/pkg/cli/tk.(*listBox).Render(0xc000372b60?, 0x7?, 0x2?)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/tk/listbox.go:83 +0x19
src.elv.sh/pkg/cli/tk.(*comboBox).Render(0xc000942c40, 0x7, 0x2)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/tk/combobox.go:51 +0x5d
src.elv.sh/pkg/cli.renderApp({0xc0007ac3a0, 0x2, 0x93bf60?}, 0x7, 0x945a80?)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/app.go:311 +0xa2
src.elv.sh/pkg/cli.(*app).redraw(0xc00015e000, 0x0)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/app.go:282 +0x586
src.elv.sh/pkg/cli.(*loop).Run(0xc00011c4c0)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/loop.go:123 +0x45
src.elv.sh/pkg/cli.(*app).ReadCode(0xc00015e000)
	/home/rsteube/Documents/development/github/elvish/pkg/cli/app.go:485 +0x47f
src.elv.sh/pkg/edit.(*Editor).ReadCode(0xc000062048?)
	/home/rsteube/Documents/development/github/elvish/pkg/edit/editor.go:116 +0x1b
src.elv.sh/pkg/shell.interact(0xc000208000, {0xc000062048, 0xc000062050, 0xc000062058}, 0xc0001cbca0)
	/home/rsteube/Documents/development/github/elvish/pkg/shell/interact.go:96 +0x6af
src.elv.sh/pkg/shell.(*Program).Run(0xc0001da120, {0xc000062048, 0xc000062050, 0xc000062058}, {0xc000024170, 0x0, 0x0})
	/home/rsteube/Documents/development/github/elvish/pkg/shell/shell.go:104 +0x28b
src.elv.sh/pkg/prog.composite.Run({0xc00008b800?, 0xc0001cbe20?, 0x6c9e0a?}, {0xc000062048, 0xc000062050, 0xc000062058}, {0xc000024170, 0x0, 0x0})
	/home/rsteube/Documents/development/github/elvish/pkg/prog/prog.go:177 +0x117
src.elv.sh/pkg/prog.Run({0xc000062048, 0xc000062050, 0xc000062058}, {0xc000024170, 0x1, 0x1}, {0xa46fc0, 0xc000012420})
	/home/rsteube/Documents/development/github/elvish/pkg/prog/prog.go:143 +0x4da
main.main()
	/home/rsteube/Documents/development/github/elvish/cmd/elvish/main.go:18 +0x14f
...
runtime error: integer divide by zero

Output of "elvish -version"

0.21.0-dev.0.20240609160321-2e527f77fdc7

Code of Conduct

rsteube avatar Jun 09 '24 21:06 rsteube

Obviously Elvish shouldn't panic but the question is what should it do when a user does something like "resize window to an unreasonably small size: 7x3"? Should it silently refuse to create a completion list box? Output an error saying the window is too small? Something else?

krader1961 avatar Jun 11 '24 06:06 krader1961

I'd say it should just silently refuse to create one. At most add a log for debugging.

On a tiling window manager it happens fairly often for a window to get squashed to a smaller than normal size for a short time. It just shouldn't crash.

rsteube avatar Jun 11 '24 06:06 rsteube

It just shouldn't crash.

I agree, and presumably everyone else does as well. What is unclear is whether silently rejecting an attempt to create a list box (or other TUI element) is the best solution. Would an explicit message such as "window size is too small" written to the terminal be preferable? Even without any rate limiting? And how would writing such a message to the terminal interact with the current prompt? The devil is in the details.

krader1961 avatar Jun 11 '24 07:06 krader1961

  1. there's no space to show a message
  2. it gets redrawn anyway when resized to normal

I don't think we need to think too deeply about this.

rsteube avatar Jun 11 '24 07:06 rsteube

Hmm there's something deeper here. In general Widget implementations assume that its Render method is never called with a height of 0, but that pre-condition is broken somewhere.

xiaq avatar Aug 08 '24 10:08 xiaq

OK, it's a simple one 🤦

https://github.com/elves/elvish/blob/3889bd05abc3a6b709c234ad658621d8fea0640a/pkg/cli/tk/combobox.go#L51

xiaq avatar Aug 08 '24 10:08 xiaq