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

Cluttered completion when window is small #1282

Open
rsteube opened this issue Apr 4, 2021 · 20 comments
Open

Cluttered completion when window is small #1282

rsteube opened this issue Apr 4, 2021 · 20 comments

Comments

@rsteube
Copy link
Contributor

rsteube commented Apr 4, 2021

This is a bit weird as the other completers work and just this one gives problems (and only when the window is small, when it's larger it works).
Should be a side effect from one of the descriptions, but i can't see what is causing it at the moment.

edit:completion:arg-completer[mv] = [@arg]{
    set opts = '[{"Value":"-b","Display":"-b (like --backup but does not accept an argument)"},{"Value":"--backup","Display":"--backup (make a backup of each existing destination file)"},{"Value":"--context","Display":"--context (set SELinux security context of destination file to default type)"},{"Value":"-Z","Display":"-Z (set SELinux security context of destination file to default type)"},{"Value":"--force","Display":"--force (do not prompt before overwriting)"},{"Value":"-f","Display":"-f (do not prompt before overwriting)"},{"Value":"--help","Display":"--help (display this help and exit)"},{"Value":"--interactive","Display":"--interactive (prompt before overwrite)"},{"Value":"-i","Display":"-i (prompt before overwrite)"},{"Value":"--no-clobber","Display":"--no-clobber (do not overwrite an existing file)"},{"Value":"-n","Display":"-n (do not overwrite an existing file)"},{"Value":"--no-target-directory","Display":"--no-target-directory (treat DEST as a normal file)"},{"Value":"-T","Display":"-T (treat DEST as a normal file)"},{"Value":"--strip-trailing-slashes","Display":"--strip-trailing-slashes (remove any trailing slashes from each SOURCE argument)"},{"Value":"--suffix","Display":"--suffix (override the usual backup suffix)"},{"Value":"-S","Display":"-S (override the usual backup suffix)"},{"Value":"--target-directory","Display":"--target-directory (move all SOURCE arguments into DIRECTORY)"},{"Value":"-t","Display":"-t (move all SOURCE arguments into DIRECTORY)"},{"Value":"--update","Display":"--update (move only when the SOURCE file is newer than the destination file or when the destination file is missing)"},{"Value":"-u","Display":"-u (move only when the SOURCE file is newer than the destination file or when the destination file is missing)"},{"Value":"--verbose","Display":"--verbose (explain what is being done)"},{"Value":"-v","Display":"-v (explain what is being done)"},{"Value":"--version","Display":"--version (output version information and exit)"}]'
    echo $opts | from-json | all (one) | each [c]{ edit:complex-candidate $c[Value] &display=$c[Display] }
}

asciicast

@zzamboni
Copy link
Contributor

zzamboni commented Apr 4, 2021

I cannot reproduce this issue, even when I make the terminal window very narrow and small it still works fine. Maybe something triggered by your right-side prompt? I don't normally use one, and a simple one did not trigger the problem but maybe something to try.

Maybe this is something specific to your terminal emulator - what are you using?

As a side comment, there's no need to store the descriptions as a string and then do the from-json part, you could store them directly as an Elvish data structure.

@rsteube
Copy link
Contributor Author

rsteube commented Apr 4, 2021

Size of the window (shown lines) seems to be very important for this. Couldn't reproduce it at first in the docker container and had to alter the prompt to take as many lines as locally to cause the error. So it seems to be important where the text is "cut".
I am using st, will try another but i don't think it is what is causing ti.
Right side prompt has never been a problem before, it's just this rare case where it happens.
The json string is normally returned from my completer, i just used a variable to reproduce the error.

@rsteube
Copy link
Contributor Author

rsteube commented Apr 4, 2021

Try edit:prompt = { tilde-abbr $pwd; put '❱ ' } and resize the terminal to show 26 (maybe 25?) lines (80 charactes width).

mv -<TAB>

@rsteube
Copy link
Contributor Author

rsteube commented Apr 4, 2021

Same in alacritty and only with the correct height (one line more/less and it works).

@zzamboni
Copy link
Contributor

zzamboni commented Apr 5, 2021

Sorry, still nothing. I tried with both Terminal.app and iTerm2 on macOS, with the prompt and sizes you suggested, and I still see the correct behavior. I still think this might be emulator-specific, or maybe something specific in Linux-based terminal emulators.

@rsteube
Copy link
Contributor Author

rsteube commented Apr 5, 2021

Just tried to force the error in windows but so far couldn't reproduce it there as well.

@rsteube
Copy link
Contributor Author

rsteube commented Apr 5, 2021

Same error in gnome-terminal, so might be linux related.

@krader1961
Copy link
Contributor

I, too, cannot reproduce using iTerm or Terminal on macOS. It's pretty unlikely to be "linux related". I initially thought it might be a bug in the VTE code but some of the terminal emulators you listed don't use the VTE library. So it's more likely to be a quirk of your environment. Such as how elvish and your terminal decide the width of a particular Unicode code point.

@rsteube: Can you reproduce this problem inside a elvish -norc session?

@rsteube
Copy link
Contributor Author

rsteube commented Apr 6, 2021

@krader1961 same with elvish -norc, default prompt and window size showing 25 lines (80 char width).

@rsteube
Copy link
Contributor Author

rsteube commented Apr 6, 2021

Same with xubuntu in virtualbox vm when resized to 25 lines:
xubuntu

@xiaq
Copy link
Member

xiaq commented Apr 6, 2021

I can reproduce this on Linux with Konsole.

Rather than this being caused by the various terminal implementations on Linux, it's more likely the other way around - the two terminal implementations on macOS happen to not have this particular problem.

@krader1961
Copy link
Contributor

TL;DR: I was able to reproduce on macOS using iTerm.

This is a bit weird as the other completers work and just this one gives problems (and only when the window is small, when it's larger it works).

What I don't understand is what "when the window is small" means given that you say you can reproduce the problem in a 80x25 terminal window. That is, more or less, the size of a classic hardware terminal (80x24 plus a status line). That is not a "small window".

I created a iTerm window using my default size and manually resized it to 80x25 using my pointer. That resulted in some unexpected artifacts from rendering the left and right prompts. That may, or may not, be relevant. Using the mv completion in your opening comment I can reproduce the problem by typing mv and pressing Tab multiple times. Setting the window height to 24 or 26 lines does not exhibit the problem. The width of the terminal is irrelevant.

This appears to be a classic "off by one" error. You can produce this by setting the terminal height to 26 lines and typing mv ^, Enter, then pressing Tab multiple times. In other words, the problem occurs when the number of options is equal to the available lines for displaying the options. The available lines is a subset of the terminal height and depends on what has already been displayed while typing a command.

@krader1961
Copy link
Contributor

Note that if you change the opts variable to have fewer (or more) Value:Display map entries than 23 that will affect the specific terminal height that exhibits the problem. For example, if I remove a single option from that map then the problem no longer occurs in a 80x25 terminal. I have to change the dimensions to 80x24 to reproduce the problem.

@rsteube
Copy link
Contributor Author

rsteube commented Apr 7, 2021

Good to know about the relation to the amount of entries. I assume the width is at least insofar important that the entries need to be shown as one per line.

Btw. during testing these other errors came up with an (unrealistic) small window:

github.com/elves/elvish/pkg/cli/prompt/prompt.go:70 +0x165
runtime error: makeslice: cap out of range

created by github.m/elves/elvish/pkgli/prompt.New
github.com
lves/elvish/pkg/cl
prompt/prompt.go:7
+0x165

runtime error: integer divide by zero

@rsteube
Copy link
Contributor Author

rsteube commented Apr 7, 2021

With one line prompt (edit:prompt = { tilde-abbr $pwd; put '❱ ' }) and 3 lines height on mv -<TAB>:

~mv -                                                                                    rsteube@mba
goroutine 1 [running]:
github.com/elves/elvish/pkg/sys.DumpStack(0xc00019ad50, 0x5572e0ffa320)
        github.com/elves/elvish/pkg/sys/dumpstack.go:10 +0x9f
github.com/elves/elvish/pkg/shell.handlePanic()
        github.com/elves/elvish/pkg/shell/interact.go:39 +0x8a
panic(0x5572e0ffa320, 0x5572e1262fe0)
        runtime/panic.go:965 +0x1b9
github.com/elves/elvish/pkg/cli.getHorizontalWindow(0x5572e1070ea0, 0xc0004354a0, 0x0, 0x0, 0x0, 0x0,
0x66, 0x1, 0xc00019b038, 0x5572e0ab77c5)
        github.com/elves/elvish/pkg/cli/listbox_window.go:119 +0x24e
github.com/elves/elvish/pkg/cli.(*listBox).renderHorizontal.func1(0xc0000603c8)
        github.com/elves/elvish/pkg/cli/listbox.go:96 +0x154
github.com/elves/elvish/pkg/cli.(*listBox).mutate(0xc000060360, 0xc00019b198)
        github.com/elves/elvish/pkg/cli/listbox.go:398 +0x72
github.com/elves/elvish/pkg/cli.(*listBox).renderHorizontal(0xc000060360, 0x66, 0x1, 0x16)
        github.com/elves/elvish/pkg/cli/listbox.go:92 +0xd4
github.com/elves/elvish/pkg/cli.(*listBox).Render(0xc000060360, 0x66, 0x1, 0xc0002d9860)
        github.com/elves/elvish/pkg/cli/listbox.go:83 +0x47
github.com/elves/elvish/pkg/cli.(*comboBox).Render(0xc0001a6d00, 0x66, 0x2, 0xc0002d97d0)
        github.com/elves/elvish/pkg/cli/combobox.go:51 +0x9b
github.com/elves/elvish/pkg/cli.renderApp(0x7f0fded1dac8, 0xc000320000, 0x7f0fded3d980, 0xc0001a6d00,
0x66, 0x3, 0x5572e0ffa5c0)
        github.com/elves/elvish/pkg/cli/app.go:241 +0xb8
github.com/elves/elvish/pkg/cli.(*app).redraw(0xc00031e000, 0x0)
        github.com/elves/elvish/pkg/cli/app.go:216 +0x54c
github.com/elves/elvish/pkg/cli.(*loop).Run(0xc000072500, 0xc00002c101, 0xc0004ac540, 0xc00031e000, 0xc0004929c0)
        github.com/elves/elvish/pkg/cli/loop.go:123 +0x5f
github.com/elves/elvish/pkg/cli.(*app).ReadCode(0xc00031e000, 0x0, 0x0, 0x0, 0x0)
        github.com/elves/elvish/pkg/cli/app.go:334 +0x3f9
github.com/elves/elvish/pkg/edit.(*Editor).ReadCode(0xc0001a6100, 0xc0000ac010, 0xc0000ac008, 0xc0000ac010, 0xc00016dae0)
        github.com/elves/elvish/pkg/edit/editor.go:99 +0x35
github.com/elves/elvish/pkg/shell.Interact(0xc0000ac000, 0xc0000ac008, 0xc0000ac010, 0xc00019bcb8)
        github.com/elves/elvish/pkg/shell/interact.go:81 +0x3c2
github.com/elves/elvish/pkg/shell.program.Run(0xc0000ac000, 0xc0000ac008, 0xc0000ac010, 0xc00026d2d0,
0xc00009e1d0, 0x0, 0x0, 0x5572e12a0a60, 0xc00019bf68)
        github.com/elves/elvish/pkg/shell/shell.go:40 +0x2de
github.com/elves/elvish/pkg/prog.Run(0xc0000ac000, 0xc0000ac008, 0xc0000ac010, 0xc00009e1d0, 0x1, 0x1, 0xc00019bf48, 0x3, 0x3, 0x0)
        github.com/elves/elvish/pkg/prog/prog.go:136 +0x24b
main.main()
        github.com/elves/elvish/main.go:17 +0x125

goroutine 33 [chan receive]:
github.com/elves/elvish/pkg/eval.getBlackholeChan.func1(0xc000246060)
        github.com/elves/elvish/pkg/eval/port.go:67 +0x49
created by github.com/elves/elvish/pkg/eval.getBlackholeChan
        github.com/elves/elvish/pkg/eval/port.go:66 +0x5a

goroutine 5 [syscall]:
os/signal.signal_recv(0x5572e1071350)
        runtime/sigqueue.go:168 +0xa5
os/signal.loop()
        os/signal/signal_unix.go:23 +0x25
created by os/signal.Notify.func1.1
        os/signal/signal.go:151 +0x46

goroutine 49 [chan receive]:
github.com/elves/elvish/pkg/shell.setupShell.func1(0xc000062120, 0xc0000ac000, 0xc0000ac008, 0xc0000ac010)
        github.com/elves/elvish/pkg/shell/shell.go:51 +0xa5
created by github.com/elves/elvish/pkg/shell.setupShell
        github.com/elves/elvish/pkg/shell/shell.go:50 +0xf9

goroutine 7 [chan receive]:
github.com/elves/elvish/pkg/cli/prompt.(*Prompt).loop(0xc000076b60)
        github.com/elves/elvish/pkg/cli/prompt/prompt.go:77 +0xca
created by github.com/elves/elvish/pkg/cli/prompt.New
        github.com/elves/elvish/pkg/cli/prompt/prompt.go:70 +0x165

goroutine 8 [chan receive]:
github.com/elves/elvish/pkg/cli/prompt.(*Prompt).loop(0xc000076c40)
        github.com/elves/elvish/pkg/cli/prompt/prompt.go:77 +0xca
created by github.com/elves/elvish/pkg/cli/prompt.New
        github.com/elves/elvish/pkg/cli/prompt/prompt.go:70 +0x165

runtime error: integer divide by zero

Execing recovery shell /bin/sh
sh-5.1$

@krader1961
Copy link
Contributor

@rsteube: A terminal with just three lines is asking for trouble from any shell that is not compatible with a dumb terminal; such as the TI Silent 700 I used in 1977 as a high school student. 😄 Which is pretty much any shell newer than the KornShell. Nonetheless, kudos for realizing that unreasonable terminal characteristics are not handled in a reasonable fashion by elvish.

@rsteube
Copy link
Contributor Author

rsteube commented Apr 8, 2021

True, but it shouldn't crash. Im using a tiling window manager and it is not uncommon for me to to have a terminal window open with only few lines visible just to enter build/test commands.

@krader1961
Copy link
Contributor

@rsteube: Agreed that elvish shouldn't crash. The question is what should it do when asked to display completions when there is insufficient room to do so. Possibly the only reasonable course of action is to ignore the request and do nothing. Or possibly do nothing other than display a BEL character (which will cause most terminals to flash and/or provide audible feedback).

@rsteube
Copy link
Contributor Author

rsteube commented Apr 8, 2021

I think this is already integrated as when the height is 2 lines i can just cycle through the completion options at cursor position.
It's just that with 3 lines it seems to try to show them and thus fails with the divide by zero error.

Given my current prompt style this seems to rather be the case that when there is exactly 1 additional line below the prompt the error occurs.

@xiaq
Copy link
Member

xiaq commented Apr 8, 2021

The panic should be easy to fix, I'll try to do it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎨Design
Development

No branches or pull requests

4 participants