Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

listbox_window: integer divide by zero #1820

Closed
1 task done
rsteube opened this issue Jun 9, 2024 · 6 comments
Closed
1 task done

listbox_window: integer divide by zero #1820

rsteube opened this issue Jun 9, 2024 · 6 comments

Comments

@rsteube
Copy link
Contributor

rsteube commented Jun 9, 2024

What happened, and what did you expect to happen?

Seems there is still a divide by zero possible:

first := selected / height * height

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

@krader1961
Copy link
Contributor

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
Copy link
Contributor Author

rsteube commented Jun 11, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

rsteube commented Jun 11, 2024

  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.

@xiaq
Copy link
Member

xiaq commented Aug 8, 2024

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
Copy link
Member

xiaq commented Aug 8, 2024

OK, it's a simple one 🤦

bufListBox := w.listBox.Render(width, height-len(buf.Lines))

@xiaq xiaq closed this as completed in 177a366 Aug 12, 2024
@github-project-automation github-project-automation bot moved this from ❓Triage to ✅Done in All Elvish issues Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants