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

Figure out what the default completion UI should be #1078

Open
andychu opened this issue Jan 22, 2022 · 7 comments
Open

Figure out what the default completion UI should be #1078

andychu opened this issue Jan 22, 2022 · 7 comments

Comments

@andychu
Copy link
Contributor

andychu commented Jan 22, 2022

I think we need to pick something "modest" which can be translated to C++. If we have color errors that might offset the lack of fanciness

Although there is also the issue that completion uses Python yield, which doesn't translate well

@andychu
Copy link
Contributor Author

andychu commented Jan 22, 2022

Also related to #460 and #459

@andychu
Copy link
Contributor Author

andychu commented Jan 22, 2022

A core issue is that we don't use GNU readline's tokenization, which is good. We use our own shell parser

However it also means we can't use the default completion display, which knows about the TERMINAL WIDTH. So we have to write our own.

If we disable the completion display hook (only using the candidate hook), then we get something like this, which is bad:

$ ls mycpp/
ls mycpp/.gitignore               ls mycpp/_tmp/                    ls mycpp/debug_pass.py            ls mycpp/gc_heap.h                ls mycpp/mylib.py                 ls mycpp/out.txt 
ls mycpp/.mypy_cache/             ls mycpp/build-steps.sh           ls mycpp/demo.sh                  ls mycpp/gc_heap_test.cc          ls mycpp/mylib.pyc                ls mycpp/pass_state.py 

That is, the entire line is displayed.

@andychu
Copy link
Contributor Author

andychu commented Jan 22, 2022

Maybe we can write a binding for this function and call it, so then the shell doesn't have to know about the width at all?


Function: void rl_display_match_list (char **matches, int len, int max)
    A convenience function for displaying a list of strings in columnar format on Readline's output stream. matches is the list of strings, in argv format, such as a list of completion matches. len is the number of strings in matches, and max is the length of the longest string in matches. This function uses the setting of print-completions-horizontally to select how the matches are displayed (see section 1.3.1 Readline Init File Syntax). When displaying completions, this function sets the number of columns used for display to the value of completion-display-width, the value of the environment variable COLUMNS, or the screen width, in that order. 

But the nice UI may want to know about the width ... how can we get it from GNU readline?

Ah it looks like we can do this:

 If an application does not want to install a SIGWINCH handler, but is still interested in the screen dimensions, it may query Readline's idea of the screen size.

Function: void rl_get_screen_size (int *rows, int *cols)
    Return Readline's idea of the terminal's size in the variables pointed to by the arguments. 

Function: void rl_reset_screen_size (void)
    Cause Readline to reobtain the screen size and recalculate its dimensions. 

@andychu
Copy link
Contributor Author

andychu commented Jan 22, 2022

TODO


Another idea.

  1. OIL_COMP_UI=minimal -- just print one line at a time, with limits. For alternative line editors.
  2. OIL_COMP_UI=bash-like -- call rl_display_match_list() and scroll the screen
  3. OIL_COMP_UI=zsh-like -- do a dance so we don't move the prompt.

@andychu
Copy link
Contributor Author

andychu commented Jan 22, 2022

Document known issue: completion on second line doesn't really work, I think because we're just parsing each line as is

$ ( ls <enter>
> <tab>  -- this completion is wrong

Hm this is sort of bad though. Maybe it can be fixed.

@andychu
Copy link
Contributor Author

andychu commented Jan 23, 2022

Hm might not need horizontal-scroll-mode ?

Because Python does a select() loop, using the "alternate GNU readline interface", which means we have a potential hook to detect if it goes off the end of the screen? hm.

https://oilshell.zulipchat.com/#narrow/stream/121539-oil-dev/topic/bash.20usage.20of.20GNU.20readline.20-.20notes

TODO: see how zsh and fish detect this condition in code.

@andychu
Copy link
Contributor Author

andychu commented Jan 30, 2022

Fixed #257 with horizontal-scroll-mode

I think this means the zsh-like UI can be the deafult?

I think the bash-like UI makes sense in case there are still some drawing problems, but I don't think it's a big priority right now.

I think the OIL_COMP_UI rather than the flag is better, and maybe we should change nice to z_readline

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

No branches or pull requests

1 participant