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

Selecta was broken by the system() rewrite #1044

Closed
rschmitt opened this Issue Aug 7, 2014 · 14 comments

Comments

Projects
None yet
7 participants
@rschmitt
Copy link

rschmitt commented Aug 7, 2014

Selecta, a fuzzy selector (sort of like a generalized CommandT), recently stopped working with Neovim. I used a git bisect to trace the issue back to 6f30b25. Up through 3d3b233, Neovim worked with selecta perfectly; now it doesn't register any keyboard input at all.

By way of background, selecta works by opening /dev/tty directly. It takes a list of choices from stdin and produces the chosen option on stdout; all other user interaction is conducted directly through /dev/tty. Integrating vim with selecta is done with a bit of vimscript:

" Run a given vim command on the results of fuzzy selecting from a given shell
" command. See usage below.
function! SelectaCommand(choice_command, selecta_args, vim_command)
  try
    silent let selection = system(a:choice_command . " | selecta " . a:selecta_args)
  catch /Vim:Interrupt/
    " Swallow the ^C so that the redraw below happens; otherwise there will be
    " leftovers from selecta on the screen
    redraw!
    return
  endtry
  redraw!
  exec a:vim_command . " " . selection
endfunction

" Find all files in all non-dot directories starting in the working directory.
" Fuzzy select one of those. Open the selected file with :e.
nnoremap <leader>f :call SelectaCommand("find * -type f", "", ":e")<cr>

Want to back this issue? Place a bounty on it! We accept bounties via Bountysource.

@justinmk justinmk added the bug label Aug 7, 2014

@justinmk justinmk added this to the first release milestone Aug 7, 2014

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Aug 7, 2014

Aw shucks. I hope I can repro on my box in the future when I get time again. But what sounds weird to me is that it requires keyboard input. I didn't think system() handled keyboard input (only fixed input in the second argument). I'm a tad bit perplexed now.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Aug 7, 2014

This is not surprising, and you presented the reason:

By way of background, selecta works by opening /dev/tty directly. It takes a list of choices from stdin and produces the chosen option on stdout; all other user interaction is conducted directly through /dev/tty. Integrating vim with selecta is done with a bit of vimscript:

The new system implementation calls the shell with all std file descriptors connected to pipes, and selecta relies on a tty being connected to it's stderr to interact with the user. From the readme:

Note that Selecta has to run in a terminal by its nature, so this Vim example will not work in graphical Vims like MacVim.

Selecta is more of a command-line utility than an editor-agnostic fuzzy matcher, in fact it assumes the following about the editor(or parent program):

  • It has a terminal UI
  • It won't try to read arbitrary keypresses or redraw the terminal while selecta is running
  • It won't redirect it's stderr(this is where compatibility with Neovim breaks)

Even if we were to add a job_start option to make it inherit Neovim stderr(and use it from os_system), it would only work until we start using UIs over msgpack.

Selecta seems to be a great tool, but it would require some changes to play nicely with Neovim

@jszakmeister

This comment has been minimized.

Copy link
Member

jszakmeister commented Aug 7, 2014

It won't redirect it's stderr(this is where compatibility with Neovim breaks)

I'm not sure that's true. selecta is accessing /dev/tty directly to render it's output--no stderr involved. You'll definitely clobber stuff on the terminal, but it's not actually coming through stderr for that. It seems more likely that this is the problem:

It won't try to read arbitrary keypresses or redraw the terminal while selecta is running

since we have both Neovim and selecta fighting over keyboard input.

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Aug 7, 2014

since we have both Neovim and selecta fighting over keyboard input.

Hmm, the event_poll on the new job_wait specifically doesn't check for keyboard input. It only checks for:

  1. output from the job
  2. signals (interrupt, terminal is incooked mode, the only kb interation that will work)

I suppose this was different for the old implementation, which checked for everything (?). But I'm not sure how selecta grabbed this input...

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Aug 7, 2014

👍 I assumed it used stderr because that's how vim interacts with the user when stdin is redirected to /dev/null

@jszakmeister

This comment has been minimized.

Copy link
Member

jszakmeister commented Aug 7, 2014

Playing around with this, if you keep hitting keys, you'll see that selecta does get some of them, and once you get enter accepted by selecta, the keys are then dumped into Vim. So something is definitely in the background fighting over keystrokes. :-)

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Aug 7, 2014

it may be caused by os_system setting the tty to cooked

@jdavis

This comment has been minimized.

Copy link
Contributor

jdavis commented Sep 9, 2014

Has there been any more investigation into this?

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Sep 9, 2014

Not yet, today I installed selecta to see what it does, and I'm gonna have to look closer at how it works under both vim and nvim. But it's not my main neovim spearhead at the moment. I've not forgotten about it though.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Sep 11, 2014

I haven't investigated it, but I'm almost sure @jszakmeister is right, nvim is fighting over keyboard with selecta. To fix it, event_poll must be patched with a boolean flag to disable the input_start/input_stop calls.

In any case, I advise against that approach because even if the problem is fixed now, selecta will eventually stop working for nvim. As I have explained before:

Selecta is more of a command-line utility than an editor-agnostic fuzzy matcher, in fact it assumes the following about the editor(or parent program):

It has a terminal UI

The above assumption will be false when nvim externalizes its UI

@justinmk

This comment has been minimized.

Copy link
Member

justinmk commented Jun 28, 2015

Closing this as "wontfix", use :term instead.

@rschmitt

This comment has been minimized.

Copy link

rschmitt commented Jul 11, 2015

Can you give an example of how to use :term instead of system in this case? It's not clear how to capture the stdout of a command, for instance.

@ZyX-I

This comment has been minimized.

Copy link
Contributor

ZyX-I commented Jul 11, 2015

@rschmitt After opening a buffer with terminal use getline() as usual for non-terminal buffers.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jul 12, 2015

Can you give an example of how to use :term instead of system in this case? It's not clear how to capture the stdout of a command, for instance.

@rschmitt you could use shell redirection and a fifo to split stdout data from a program running under :term, here's an idea: #2836 (comment)

A simpler solution may be to use a temporary file and read it after the terminal program has exited, see fzf

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