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

fish incorrectly completes after escaped space character #2447

Closed
tclementdev opened this Issue Sep 30, 2015 · 8 comments

Comments

Projects
None yet
6 participants
@tclementdev

Enter the following:

sudo /Applications/Font\<space here>

Then hit tab, fish offers completions for the currently directory:

sudo /Applications/Font\ Book.app/
$ns/Font\ Applications/  (Command to run)  $ns/Font\ Library/   (Command to run)  $ns/Font\ Public/  (Command to run)
$ns/Font\ Desktop/       (Command to run)  $ns/Font\ Movies/    (Command to run)  $ns/Font\ Sites/   (Command to run)
$ns/Font\ Documents/     (Command to run)  $ns/Font\ Music/     (Command to run)  
$ns/Font\ Downloads/     (Command to run)  $ns/Font\ Pictures/  (Command to run)

Instead of:

sudo /Applications/Font\ Book.app/
@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 30, 2015

Member

Sooo... You only get this when you press tab right after the escaped (or quoted) space, right?

I.e. sudo /Applications/Font\ <TAB> causes issues, sudo /Applications/Font<TAB> doesn't?

Member

faho commented Sep 30, 2015

Sooo... You only get this when you press tab right after the escaped (or quoted) space, right?

I.e. sudo /Applications/Font\ <TAB> causes issues, sudo /Applications/Font<TAB> doesn't?

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 30, 2015

Member

Compare also #2274.

Member

faho commented Sep 30, 2015

Compare also #2274.

@tclementdev

This comment has been minimized.

Show comment
Hide comment
@tclementdev

tclementdev Sep 30, 2015

Yes, sudo /Applications/Font\ <TAB> causes the issue, sudo /Applications/Font<TAB> doesn't.

Yes, sudo /Applications/Font\ <TAB> causes the issue, sudo /Applications/Font<TAB> doesn't.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 30, 2015

Member

I've investigated this a bit and found there's two issues at play here:

  • complete --do-complete expects the argument to be double-escaped (i.e. passed as one still escaped argument) - this means if you run complete -C "/Applications/Font " you'll get a completion as if the cursor was after a space - i.e. by default a file

Now, this is undoubtedly useful in many cases, and it's quite clean - it accepts a commandline, so you can just hand it a commandline (or part of one) and it'll do the right thing. Well, except it won't since commandline breaks quoting (#2210) and also escaping - if I have echo "echo banana" > a\ b in my commandline, it outputs echo echo banana > a b. But if commandline were bugfree, it'd work.

This part is worked around easy enough - just use string escape -n.

However, there's a second part here, and it's one I've mentioned in a previous issue (though I can't find the bugger):

  • Even when we output the correct string , the comparison logic is off so it won't be properly inserted

This is probably better explained via example:

If I make it output the normal string (i.e. /Applications/Font Book.app), what ends up in the commandline is /Applications/Font\ /Applications/Font\ Book.app/.
If I make the completion output the escaped string (i.e. /Applications/Font\ Book.app/), what ends up in the commandline is /Applications/Font\ /tmp/Font\\\ Book.app/.

If one manages to trigger the completion menu (because there's more than one choice), one can pick both and they correctly replace the commandline.

As I've said before - completion before the space works.

@ridiculousfish: Mind taking a look? I think I've narrowed it down a bit.

Member

faho commented Sep 30, 2015

I've investigated this a bit and found there's two issues at play here:

  • complete --do-complete expects the argument to be double-escaped (i.e. passed as one still escaped argument) - this means if you run complete -C "/Applications/Font " you'll get a completion as if the cursor was after a space - i.e. by default a file

Now, this is undoubtedly useful in many cases, and it's quite clean - it accepts a commandline, so you can just hand it a commandline (or part of one) and it'll do the right thing. Well, except it won't since commandline breaks quoting (#2210) and also escaping - if I have echo "echo banana" > a\ b in my commandline, it outputs echo echo banana > a b. But if commandline were bugfree, it'd work.

This part is worked around easy enough - just use string escape -n.

However, there's a second part here, and it's one I've mentioned in a previous issue (though I can't find the bugger):

  • Even when we output the correct string , the comparison logic is off so it won't be properly inserted

This is probably better explained via example:

If I make it output the normal string (i.e. /Applications/Font Book.app), what ends up in the commandline is /Applications/Font\ /Applications/Font\ Book.app/.
If I make the completion output the escaped string (i.e. /Applications/Font\ Book.app/), what ends up in the commandline is /Applications/Font\ /tmp/Font\\\ Book.app/.

If one manages to trigger the completion menu (because there's more than one choice), one can pick both and they correctly replace the commandline.

As I've said before - completion before the space works.

@ridiculousfish: Mind taking a look? I think I've narrowed it down a bit.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Jan 20, 2016

Member

Moved @ridiculousfish's self-assignment to this one from #2687.

Member

faho commented Jan 20, 2016

Moved @ridiculousfish's self-assignment to this one from #2687.

@krader1961 krader1961 modified the milestones: next-2.x, 2.3.0 Mar 22, 2016

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Mar 22, 2016

Contributor

We've only got a handful of issues blocking the 2.3.0 release milestone. This issue isn't one that should block a new minor release so I'm retargeting this to next-2.x so we can focus our efforts on fixing the issues that should be fixed for the 2.3.0 release.

Contributor

krader1961 commented Mar 22, 2016

We've only got a handful of issues blocking the 2.3.0 release milestone. This issue isn't one that should block a new minor release so I'm retargeting this to next-2.x so we can focus our efforts on fixing the issues that should be fixed for the 2.3.0 release.

@lesderid

This comment has been minimized.

Show comment
Hide comment
@lesderid

lesderid May 19, 2016

This is the only bug I experience with fish, but it happens on a daily basis. It doesn't make fish unusable for general use, but it's not very user friendly.

This should probably get the 'bug' tag.

This is the only bug I experience with fish, but it happens on a daily basis. It doesn't make fish unusable for general use, but it's not very user friendly.

This should probably get the 'bug' tag.

@faho faho added the bug label May 19, 2016

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jul 11, 2016

Member

Fixed as f08ac96. Thanks for filing this.

Member

ridiculousfish commented Jul 11, 2016

Fixed as f08ac96. Thanks for filing this.

@krader1961 krader1961 modified the milestones: fish 2.4.0, next-2.x Sep 3, 2016

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