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

Make completions not block #23

Open
ridiculousfish opened this Issue Jun 1, 2012 · 17 comments

Comments

Projects
None yet
7 participants
@ridiculousfish
Member

ridiculousfish commented Jun 1, 2012

From http://comments.gmane.org/gmane.comp.shells.fish.user/3081:

"Consider this foo.fish file:

complete --command foo --no-files --short-option a --long-option 'an_example' --description 'example A'
complete --command foo --no-files --arguments '(sleep 5s)' --description 'slept'

Type in the shell

foo -

and press tab. 5 seconds pass, before we get the result. If I delete the second line from foo.fish, the result is instantanious. AFAIK, the --arguments should not be executed when fish is completing options for a command."

It would be nice if completions were not blocking. Perhaps they could be executed in a cancelable subshell.

lgarron added a commit to lgarron/dotfiles that referenced this issue Jun 30, 2016

Stub out some git completions that are way to slow in the Chromium repo.
Until fish-shell/fish-shell#23 is implemented, there
is no workaroudn to prevent the shell from hanging.

lgarron added a commit to lgarron/dotfiles that referenced this issue Jun 30, 2016

[chrome.fish] Stub out some git completions that are way to slow in t…
…he Chromium repo.

Until fish-shell/fish-shell#23 is implemented, there
is no workaroudn to prevent the shell from hanging.

lgarron added a commit to lgarron/dotfiles that referenced this issue Jun 30, 2016

[chrome.fish] Stub out some git completions that are way to slow in t…
…he Chromium repo.

Until fish-shell/fish-shell#23 is implemented, there
is no workaround to prevent the shell from hanging.

@floam floam changed the title from Slow completions can hang fish to Make completions not block Sep 1, 2016

@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Aug 8, 2017

Contributor

So to comment on a really old open issue... what would the correct behavior be in this case? The completion can obviously easily be evaluated in the background, but what happens in meantime? Does the completion abort if the user enters another key? Is the result computed regardless and cached for next use? Are results provided by the completion source added asynchronously in real-time to the GUI (which updates each time a new argument is computed) and then removed (also in real-time) as they are "ruled out" by further input from the user?

Contributor

mqudsi commented Aug 8, 2017

So to comment on a really old open issue... what would the correct behavior be in this case? The completion can obviously easily be evaluated in the background, but what happens in meantime? Does the completion abort if the user enters another key? Is the result computed regardless and cached for next use? Are results provided by the completion source added asynchronously in real-time to the GUI (which updates each time a new argument is computed) and then removed (also in real-time) as they are "ruled out" by further input from the user?

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Aug 8, 2017

Contributor

FWIW, I had the same reaction as @mqudsi when I first read this issue. I was simply too scared to make a similar comment since I was a new core contributor. The most important question is how to provide predictable behavior. Because humans are really annoyed by unpredictable behavior.

It is one thing to avoid blocking fish non interactive behavior because NFS or some other networked file system is slow. As in my fix for issue #685 involving updating the command history file. It is quite another thing to arbitrarily succeed or fail a completion because of an arbitrary time limit.

Contributor

krader1961 commented Aug 8, 2017

FWIW, I had the same reaction as @mqudsi when I first read this issue. I was simply too scared to make a similar comment since I was a new core contributor. The most important question is how to provide predictable behavior. Because humans are really annoyed by unpredictable behavior.

It is one thing to avoid blocking fish non interactive behavior because NFS or some other networked file system is slow. As in my fix for issue #685 involving updating the command history file. It is quite another thing to arbitrarily succeed or fail a completion because of an arbitrary time limit.

@gkatsanos

This comment has been minimized.

Show comment
Hide comment
@gkatsanos

gkatsanos Sep 26, 2017

So, no fix for completions performance yet I guess? pity, it is otherwise a nice shell..

gkatsanos commented Sep 26, 2017

So, no fix for completions performance yet I guess? pity, it is otherwise a nice shell..

@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Sep 26, 2017

Contributor

@gkatsanos what completions are particularly slow for you?

Contributor

mqudsi commented Sep 26, 2017

@gkatsanos what completions are particularly slow for you?

@gkatsanos

This comment has been minimized.

Show comment
Hide comment
@gkatsanos

gkatsanos Sep 26, 2017

@mqudsi autocomplete git branches

gkatsanos commented Sep 26, 2017

@mqudsi autocomplete git branches

@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Sep 26, 2017

Contributor

That's been fixed in 3.0 and, I think, the current 2.7.0 branch. Have you given it a try?

Contributor

mqudsi commented Sep 26, 2017

That's been fixed in 3.0 and, I think, the current 2.7.0 branch. Have you given it a try?

@gkatsanos

This comment has been minimized.

Show comment
Hide comment
@gkatsanos

gkatsanos Sep 26, 2017

@mqudsi unfortunately the latest on brew seems to be 2.6.x

~/D/r/ucb-client (BAC-4346-adjust-search-widget-button-size|✚1) $
brew upgrade fish
Updating Homebrew...
Error: fish 2.6.0 already installed
~/D/r/ucb-client (BAC-4346-adjust-search-widget-button-size|✚1) $
fish --version
fish, version 2.6.0

gkatsanos commented Sep 26, 2017

@mqudsi unfortunately the latest on brew seems to be 2.6.x

~/D/r/ucb-client (BAC-4346-adjust-search-widget-button-size|✚1) $
brew upgrade fish
Updating Homebrew...
Error: fish 2.6.0 already installed
~/D/r/ucb-client (BAC-4346-adjust-search-widget-button-size|✚1) $
fish --version
fish, version 2.6.0
@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Sep 26, 2017

Contributor

Yes, neither is released yet (that's why I said "branch" - you'll have to get them from git).

I do believe homebrew has an option to build from git/source, though I ditched Mac a few years back. You can try --head if my memory does not deceive me.

Contributor

mqudsi commented Sep 26, 2017

Yes, neither is released yet (that's why I said "branch" - you'll have to get them from git).

I do believe homebrew has an option to build from git/source, though I ditched Mac a few years back. You can try --head if my memory does not deceive me.

@gkatsanos

This comment has been minimized.

Show comment
Hide comment
@gkatsanos

gkatsanos Sep 26, 2017

Actually in the fish website 2.6 seems the latest available https://fishshell.com/

gkatsanos commented Sep 26, 2017

Actually in the fish website 2.6 seems the latest available https://fishshell.com/

@gkatsanos

This comment has been minimized.

Show comment
Hide comment
@gkatsanos

gkatsanos Sep 26, 2017

Ah I see, I 'll wait it out, I don't wanna risk breaking now that it's working properly :) any idea of when the release is gonna happen?

gkatsanos commented Sep 26, 2017

Ah I see, I 'll wait it out, I don't wanna risk breaking now that it's working properly :) any idea of when the release is gonna happen?

@hamon-e

This comment has been minimized.

Show comment
Hide comment
@hamon-e

hamon-e Feb 2, 2018

Contributor

Does someone began to work on that ?

Contributor

hamon-e commented Feb 2, 2018

Does someone began to work on that ?

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Feb 2, 2018

Member

I'm not aware of anyone, no. And there's a few open questions still - see @mqudsi's comment above.

Member

faho commented Feb 2, 2018

I'm not aware of anyone, no. And there's a few open questions still - see @mqudsi's comment above.

@hamon-e

This comment has been minimized.

Show comment
Hide comment
@hamon-e

hamon-e Feb 2, 2018

Contributor

Well as i see it:
The completion should not abort if the user enters another key but only if the "token" change
Otherwise the result could be cache, and once the result found, only perfom a new search to match the difference that have may occurred in the token

I think it would be kinda weird to have the completion showing in real-time, we could show them when everything has been computed

What do you think of it ?

Contributor

hamon-e commented Feb 2, 2018

Well as i see it:
The completion should not abort if the user enters another key but only if the "token" change
Otherwise the result could be cache, and once the result found, only perfom a new search to match the difference that have may occurred in the token

I think it would be kinda weird to have the completion showing in real-time, we could show them when everything has been computed

What do you think of it ?

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Feb 8, 2018

Member

The completion should not abort if the user enters another key but only if the "token" change

So anything that removes or adds any characters should abort? So I press "f" and that aborts, but I press right-arrow, and that doesn't?

Otherwise the result could be cache, and once the result found, only perfom a new search to match the difference that have may occurred in the token

You just said if the token changes it should abort? I'm afraid I don't quite understand this.

I think it would be kinda weird to have the completion showing in real-time, we could show them when everything has been computed

I agree that having completions showing up "in real-time", i.e. not all at ones (like now) would be weird. Especially when you consider sorting - currently we sort completions (by default), so we'd either have completions appear in the middle of the pager (possibly under the cursor or where you're just moving), or we'd have this half-sorted list.

However, having completions show up when everything has been computed is what happens now, so what exactly should change here?

Right now, you can only press ctrl-c to abort completions. Do you just want to change that so that more keys do?

Member

faho commented Feb 8, 2018

The completion should not abort if the user enters another key but only if the "token" change

So anything that removes or adds any characters should abort? So I press "f" and that aborts, but I press right-arrow, and that doesn't?

Otherwise the result could be cache, and once the result found, only perfom a new search to match the difference that have may occurred in the token

You just said if the token changes it should abort? I'm afraid I don't quite understand this.

I think it would be kinda weird to have the completion showing in real-time, we could show them when everything has been computed

I agree that having completions showing up "in real-time", i.e. not all at ones (like now) would be weird. Especially when you consider sorting - currently we sort completions (by default), so we'd either have completions appear in the middle of the pager (possibly under the cursor or where you're just moving), or we'd have this half-sorted list.

However, having completions show up when everything has been computed is what happens now, so what exactly should change here?

Right now, you can only press ctrl-c to abort completions. Do you just want to change that so that more keys do?

@hamon-e

This comment has been minimized.

Show comment
Hide comment
@hamon-e

hamon-e Feb 9, 2018

Contributor

I used the word token but was more thinking about word
imagine you type ls somefold<TAB>, the point would be to begin the search of all the folder, let you type want you want next, and when you found all the folder you're only showing the one who match with you're updated research

Contributor

hamon-e commented Feb 9, 2018

I used the word token but was more thinking about word
imagine you type ls somefold<TAB>, the point would be to begin the search of all the folder, let you type want you want next, and when you found all the folder you're only showing the one who match with you're updated research

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Jun 13, 2018

Member

Some completions actually are cancellable for me, with ctrl-c.

I would imagine that's a side-effect of #3922, which would mean that ctrl-c is sent to whatever command the completion is running, so it would have to quit. That's not completely ideal but should work in the vast majority of cases.

Cancelling internal completions like ** also works for me, but @sabinem reports in #5045 that that's not the case for everyone.

Member

faho commented Jun 13, 2018

Some completions actually are cancellable for me, with ctrl-c.

I would imagine that's a side-effect of #3922, which would mean that ctrl-c is sent to whatever command the completion is running, so it would have to quit. That's not completely ideal but should work in the vast majority of cases.

Cancelling internal completions like ** also works for me, but @sabinem reports in #5045 that that's not the case for everyone.

@mqudsi

This comment has been minimized.

Show comment
Hide comment
@mqudsi

mqudsi Jun 13, 2018

Contributor

I have been playing around with a concept for the UI for this, temporarily powered by fzf: http://share.neosmart.net/View/Index/p9Dbjd.fish

The UX is not too bad, though I need to figure out how to use fzf's key binding to get tab to navigate between completions, and it's missing support for descriptions and multiple columns. (I had to remove descriptions because fzf would try to match input against the descriptions, too.)

For this to work right, filtering shouldn't be done in fish, everything should be piped to the completion program and that's where filtering should be done (which will speed up things on our end, anyway), allowing it to dynamically decide what to show based off of the current token. All complete providers should be run asynchronously and as they return output, it should be piped to the completion agent in realtime, which would be running from as soon as tab is first pressed.

One catch is with ctrl+c behavior. Obviously this fundamentally fixes the underlying problem if all commands are ultimately piped to the frontend, but with this fzf plugin once the completion UI is presented the first ctrl+c closes the ui and it takes a second ctrl+c to clear the line.

Contributor

mqudsi commented Jun 13, 2018

I have been playing around with a concept for the UI for this, temporarily powered by fzf: http://share.neosmart.net/View/Index/p9Dbjd.fish

The UX is not too bad, though I need to figure out how to use fzf's key binding to get tab to navigate between completions, and it's missing support for descriptions and multiple columns. (I had to remove descriptions because fzf would try to match input against the descriptions, too.)

For this to work right, filtering shouldn't be done in fish, everything should be piped to the completion program and that's where filtering should be done (which will speed up things on our end, anyway), allowing it to dynamically decide what to show based off of the current token. All complete providers should be run asynchronously and as they return output, it should be piped to the completion agent in realtime, which would be running from as soon as tab is first pressed.

One catch is with ctrl+c behavior. Obviously this fundamentally fixes the underlying problem if all commands are ultimately piped to the frontend, but with this fzf plugin once the completion UI is presented the first ctrl+c closes the ui and it takes a second ctrl+c to clear the line.

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