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

Tab completions shouldn't move the cursor when there are no completions #4128

Closed
wants to merge 2 commits into from
Closed

Tab completions shouldn't move the cursor when there are no completions #4128

wants to merge 2 commits into from

Conversation

rolag
Copy link
Contributor

@rolag rolag commented Jun 15, 2017

Description

This fixes #4124
This fixes the first example, mpv --|'https://...very-very-long-url...'. I'm not quite sure how to test it, however, because it changes the reader.cpp loop, which doesn't seem to have any tests in fish_tests.cpp.

The second example, foo /path-typo|:/usr/lib, is much more complicated and seems to be because the function completer_t::complete_param_expand() in complete.cpp doesn't know the cursor's position, and all surrounding code, including the main complete() function, assumes completions are done at the end of a token.
I looked into this a little bit and made a WIP branch complete_param_expand_enhancement_wip and it's possible to have the cursor not move when there are no completions and even expand paths before the : too. But with my current implementation I don't think it's possible to do this and move the cursor to the end when there is a completion e.g. fish --ver|si -> fish --version|, which is probably a big breaking change.

The name `comp_empty` suggests that it's true whenever the vector `comp`
is empty. However, it's actually the exact opposite because
handle_completions() returns true if it inserted text into the command
line.
@faho faho added this to the fish 2.7.0 milestone Jun 19, 2017
@faho
Copy link
Member

faho commented Jun 20, 2017

I'm not quite sure how to test it, however, because it changes the reader.cpp loop, which doesn't seem to have any tests in fish_tests.cpp.

This is something that you want to test with expect. See e.g. bind.expect in tests/.

But with my current implementation I don't think it's possible to do this and move the cursor to the end when there is a completion e.g. fish --ver|si -> fish --version|, which is probably a big breaking change

That's something we definitely do not want to change. When a completion is accepted, the cursor should be moved to the end.


What I don't like particularly is that this moves the cursor to the end, repaints and then moves it back. That's kinda visually jarring. Have you tried skipping the cursor movement entirely if there is no completion?

@faho
Copy link
Member

faho commented Jul 2, 2017

This currently doesn't work as well as it should, so I'm closing this for now.

@faho faho closed this Jul 2, 2017
@zanchey zanchey removed this from the fish 2.7.0 milestone Jul 29, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab completions shouldn't move the cursor when there are no completions
3 participants