cd completion spews on receiving [ #562

Closed
ridiculousfish opened this Issue Feb 5, 2013 · 13 comments

Comments

Projects
None yet
7 participants
@ridiculousfish
Member

ridiculousfish commented Feb 5, 2013

> cd /tmp/[<tab>

Tokenizer error: 'Unexpected end of string, parenthesis do not match'
/usr/local/share/fish/functions/__fish_complete_cd.fish (line 7): begin; printf \%s\n /tmp/[*/ ;end eval2_inner <&3 3<&-"
@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Feb 11, 2013

Member

In __fish_complete_cd.fish:

eval printf '\%s\\n' $ctoken\*/

$ctoken is /tmp/[. Passing this to eval causes the parser to complain.

Member

ridiculousfish commented Feb 11, 2013

In __fish_complete_cd.fish:

eval printf '\%s\\n' $ctoken\*/

$ctoken is /tmp/[. Passing this to eval causes the parser to complain.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Feb 11, 2013

Member

I'm not sure how to fix this; perhaps eval should have a flag that suppresses parse errors.

Member

ridiculousfish commented Feb 11, 2013

I'm not sure how to fix this; perhaps eval should have a flag that suppresses parse errors.

@LNunley

This comment has been minimized.

Show comment
Hide comment
@LNunley

LNunley Feb 6, 2014

Can this issue be fixed soon? I'm having this issue on my Raspberry Pi while trying to set up some stuff and it's pretty distracting. Is there a flag implemented to suppress parse errors?

LNunley commented Feb 6, 2014

Can this issue be fixed soon? I'm having this issue on my Raspberry Pi while trying to set up some stuff and it's pretty distracting. Is there a flag implemented to suppress parse errors?

@kballard

This comment has been minimized.

Show comment
Hide comment
@kballard

kballard Sep 24, 2014

Contributor

@ridiculousfish I don't think it's appropriate for completion to ever eval the command-line. That also runs command substitutions. While that is technically necessary to complete for real, I think it's acceptable to have completions simply not work in the face of command substitutions. And I think it's surprising for command substitutions to be evaluated upon merely hitting .

In order to support variables and globbing, which are still nice to use, fish should instead just have a builtin that expands those without expanding command substitutions.

Contributor

kballard commented Sep 24, 2014

@ridiculousfish I don't think it's appropriate for completion to ever eval the command-line. That also runs command substitutions. While that is technically necessary to complete for real, I think it's acceptable to have completions simply not work in the face of command substitutions. And I think it's surprising for command substitutions to be evaluated upon merely hitting .

In order to support variables and globbing, which are still nice to use, fish should instead just have a builtin that expands those without expanding command substitutions.

@kballard kballard added the bug label Sep 24, 2014

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Sep 25, 2014

Member

Yeah, the cd completion is terrifying.

Member

ridiculousfish commented Sep 25, 2014

Yeah, the cd completion is terrifying.

ShadowKyogre added a commit to ShadowKyogre/dotfiles that referenced this issue Sep 28, 2014

faho added a commit that referenced this issue Oct 7, 2015

Rewrite __fish_complete_cd
This no longer uses "eval" (which is scary), and is a bit shorter (which
is nice).

Fixes #2299
Fixes #952

Improves #2300
Improves #562
@smarek

This comment has been minimized.

Show comment
Hide comment
@smarek

smarek Mar 7, 2016

Could you please move this bug upper on priority ladder? It's already 3 years old and number of duplicates closed is growing. AFAICT in 2.2.0 this is still not fixed, am I right?

smarek commented Mar 7, 2016

Could you please move this bug upper on priority ladder? It's already 3 years old and number of duplicates closed is growing. AFAICT in 2.2.0 this is still not fixed, am I right?

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Mar 7, 2016

Member

This may have been fixed by 0a99772, I can't reproduce it any more. @faho do you think there's anything left to do here?

Member

ridiculousfish commented Mar 7, 2016

This may have been fixed by 0a99772, I can't reproduce it any more. @faho do you think there's anything left to do here?

@smarek

This comment has been minimized.

Show comment
Hide comment
@smarek

smarek Mar 7, 2016

I have installed 2.2.0 from Homebrew (OSX) and I can still reproduce that, should I go with HEAD version?

smarek commented Mar 7, 2016

I have installed 2.2.0 from Homebrew (OSX) and I can still reproduce that, should I go with HEAD version?

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Mar 7, 2016

Member

It doesn't spew that error anymore and handles mostly okay - the issue happens when you enter "[" manually.

If you have a directory called "[foobar]", enter "[foo" and then complete, it will complete to "[foobar]", which is actually a valid way to express that directory name. If you have a directory name without the closing bracket, it won't escape anything if you've manually entered it (picking via pager is okay).

Do we have a bug about that? It's kinda the same thing as #2300 - our completion doesn't always escape properly.

should I go with HEAD version?

@smarek: I'm biased and I'm saying yes. It's improved at least.

Member

faho commented Mar 7, 2016

It doesn't spew that error anymore and handles mostly okay - the issue happens when you enter "[" manually.

If you have a directory called "[foobar]", enter "[foo" and then complete, it will complete to "[foobar]", which is actually a valid way to express that directory name. If you have a directory name without the closing bracket, it won't escape anything if you've manually entered it (picking via pager is okay).

Do we have a bug about that? It's kinda the same thing as #2300 - our completion doesn't always escape properly.

should I go with HEAD version?

@smarek: I'm biased and I'm saying yes. It's improved at least.

@smarek

This comment has been minimized.

Show comment
Hide comment
@smarek

smarek Mar 7, 2016

In my case, i'm talking about directories with & (ampersand) in name, I've reported this back in 2015 in #2242

smarek commented Mar 7, 2016

In my case, i'm talking about directories with & (ampersand) in name, I've reported this back in 2015 in #2242

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Mar 7, 2016

Member

That case works now,

Member

faho commented Mar 7, 2016

That case works now,

@zanchey zanchey modified the milestones: 2.3.0, fish-future Mar 9, 2016

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Mar 9, 2016

Member

Sounds like this is solved - hooray!

Member

zanchey commented Mar 9, 2016

Sounds like this is solved - hooray!

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Mar 22, 2016

Contributor

I can't reproduce any of the problems reported here so I'm closing this so we've got a better idea of what's left to complete for the 2.3.0 release.

Contributor

krader1961 commented Mar 22, 2016

I can't reproduce any of the problems reported here so I'm closing this so we've got a better idea of what's left to complete for the 2.3.0 release.

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