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

completions occasionally bomb out with "complete: -d requires a non-empty string" #3557

Closed
c22 opened this Issue Nov 16, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@c22

c22 commented Nov 16, 2016

  • Have you checked if problem occurs with fish 2.4.0?
  • Tried fish without third-party customizations (check sh -c 'env HOME=$(mktemp -d) fish')?

fish version installed (fish --version):
fish, version 2.4.0
OS/terminal used:
OSX El Capitan Version 10.11.6
iTerm2 3.0.10

I alluded to this bug in another issue. I still have not been able to reliably reproduce this, best I can tell it just happens if you type too fast sometimes.

Just now I typed m l<TAB>

Results

m lcomplete: -d requires a non-empty string

It's not specific to a particular completion, I have seen it happen with others.

It also doesn't seem to be to do with custom completions or generated completions, it seems related to auto-completions from shell history.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Nov 16, 2016

Member

Can you try to use fish started as fish -d3 and see if you can reproduce it, then share the log?

Member

floam commented Nov 16, 2016

Can you try to use fish started as fish -d3 and see if you can reproduce it, then share the log?

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 16, 2016

Contributor

That error can occur in only one way AFAICT: a complete ... -d '' command was run. Do you have a m.fish completion script? What does it contain?

Contributor

krader1961 commented Nov 16, 2016

That error can occur in only one way AFAICT: a complete ... -d '' command was run. Do you have a m.fish completion script? What does it contain?

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Nov 16, 2016

Member

My guess is there is some completion with a -d $foo or --description $foo.

Member

floam commented Nov 16, 2016

My guess is there is some completion with a -d $foo or --description $foo.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 16, 2016

Contributor

Yep, that's my guess as well. Or perhaps the equivalent complete -c m -d (some_command_outputing_an_empty_string). Also, @c22, if m is a function that wraps another command then we're interested in the completion script for the wrapped command.

Contributor

krader1961 commented Nov 16, 2016

Yep, that's my guess as well. Or perhaps the equivalent complete -c m -d (some_command_outputing_an_empty_string). Also, @c22, if m is a function that wraps another command then we're interested in the completion script for the wrapped command.

@c22

This comment has been minimized.

Show comment
Hide comment
@c22

c22 Nov 16, 2016

m refers to m-cli for macOS, it turns out there is a vendor completion file that I hadn't noticed before. As mentioned above, I am fairly certain I have seen this behaviour with utilities other than m-cli but I could be mistaken.

https://raw.githubusercontent.com/rgcr/m-cli/v0.1.9/completion/fish/m.fish is the file I have in my release, I can see there is one occurrence of complete ... -d '' in the completion file, though I'm not sure why I'm only sometimes running into this.

@rgcr this might be something you can help with?

c22 commented Nov 16, 2016

m refers to m-cli for macOS, it turns out there is a vendor completion file that I hadn't noticed before. As mentioned above, I am fairly certain I have seen this behaviour with utilities other than m-cli but I could be mistaken.

https://raw.githubusercontent.com/rgcr/m-cli/v0.1.9/completion/fish/m.fish is the file I have in my release, I can see there is one occurrence of complete ... -d '' in the completion file, though I'm not sure why I'm only sometimes running into this.

@rgcr this might be something you can help with?

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Nov 16, 2016

Member

That's definitely going to be what is causing your error. Removing -d '' there will fix it.

Member

floam commented Nov 16, 2016

That's definitely going to be what is causing your error. Removing -d '' there will fix it.

@c22

This comment has been minimized.

Show comment
Hide comment
@c22

c22 Nov 16, 2016

I'm a bit puzzled as to why this only shows up sometimes and why it shows up for completions that don't have blank descriptions; seems a bit inconsistent but I'm happy for this to be closed and addressed via m-cli.

c22 commented Nov 16, 2016

I'm a bit puzzled as to why this only shows up sometimes and why it shows up for completions that don't have blank descriptions; seems a bit inconsistent but I'm happy for this to be closed and addressed via m-cli.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Nov 16, 2016

Member

What I don't get here is why we print an error in this case?

To me it seems like an empty description should just be a noop.

Member

faho commented Nov 16, 2016

What I don't get here is why we print an error in this case?

To me it seems like an empty description should just be a noop.

@faho faho reopened this Nov 16, 2016

rgcr added a commit to rgcr/m-cli that referenced this issue Nov 16, 2016

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 16, 2016

Contributor

The author of that bit of code probably expected that if there was no description you would just omit the -d/--description flag. And if you did specify that flag you probably made an inadvertent mistake if you didn't provide a description. I agree that is a silly restriction which should be removed. Also, the option parsing for that command is also atypical in how it handles flags with missing mandatory values.

Contributor

krader1961 commented Nov 16, 2016

The author of that bit of code probably expected that if there was no description you would just omit the -d/--description flag. And if you did specify that flag you probably made an inadvertent mistake if you didn't provide a description. I agree that is a silly restriction which should be removed. Also, the option parsing for that command is also atypical in how it handles flags with missing mandatory values.

@krader1961 krader1961 added this to the fish 2.5.0 milestone Nov 17, 2016

@krader1961 krader1961 self-assigned this Nov 17, 2016

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