listbox_window: integer divide by zero

rsteube commented 3 weeks ago

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

[X] I agree to follow Elvish's Code of Conduct.

krader1961 commented 3 weeks ago

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?

rsteube commented 3 weeks ago

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.

krader1961 commented 3 weeks ago

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.

rsteube commented 3 weeks ago

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

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

elves / elvish